Surface GSAS-II refinement errors; fix null-table crash in run() - #3
Open
Daniel Olds (DanOlds) wants to merge 2 commits into
Open
Surface GSAS-II refinement errors; fix null-table crash in run()#3Daniel Olds (DanOlds) wants to merge 2 commits into
Daniel Olds (DanOlds) wants to merge 2 commits into
Conversation
…esponses
Early server-side failures serialize unit_cell_data/peak_list_data as
explicit nulls; .get(key, {}) passes None through and None.items() raised
'NoneType' object has no attribute 'items' client-side, masking the real
refinement error. Found debugging a live PowderLLM session (server request
completed in 0.3s; client crashed normalizing the failure payload).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… no-Rwp G2Project.refine() discards G2strMain.Refine's (OK, Rvals) return, so errors like 'Invalid metric tensor for phase #0 — check for refinement of conflicting variables' reached only the console; callers (and any agent driving run()) saw only 'produced no Rwp'. New _refine_with_message mirrors the scriptable wrapper's non-sequential steps to keep Rvals['msg'], with a fallback to the plain proj.refine() if GSAS-II internals change. Verified against a live failure case (CeO2, cell+scale fit diverging on a restricted range). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR improves error handling and robustness in the GSAS-II engine’s run() path so client code receives the real refinement failure reason and avoids a secondary crash during result normalization.
Changes:
- Preserve and surface GSAS-II refinement failure messages by introducing
_refine_with_message()and using it inexecute_rietveld_refinement(). - Fix a client-side crash when early-failure responses serialize
unit_cell_data/peak_list_dataas explicit JSONnullby normalizing with(result.get(...) or {}). - Regenerate
_code_hash.jsonto reflectkicker.pychanges.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/powderline/kicker.py |
Capture/reforward GSAS-II failure messages and harden result normalization against null tables. |
src/powderline/_code_hash.json |
Update kicker hash after modifying kicker.py. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+2868
to
+2872
| except Exception: | ||
| pass # GSAS-II internals changed: fall back to the plain call below | ||
|
|
||
| proj.refine() | ||
| return True, '' |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two client-visible bug fixes in the GSAS-II engine's
run()path, found while debugging a live PowderLLM session. Deliberately kept off PR #2 (which promised a byte-identical kicker) and submitted separately.run()normalization crashed on failure responses (caa49d6): early server-side failures serializeunit_cell_data/peak_list_dataas explicit JSON nulls;.get(key, {})passesNonethrough andNone.items()raised'NoneType' object has no attribute 'items'client-side — masking the real refinement error. Now(result.get(...) or {}).Silent refinement failures now carry GSAS-II's message (
2d8b26c):G2Project.refine()discardsG2strMain.Refine's(OK, Rvals)return, so errors like "Invalid metric tensor for phase #0 — check for refinement of conflicting variables" reached only the console; callers saw a generic "produced no Rwp". A new_refine_with_messagemirrors the scriptable wrapper's non-sequential steps to keepRvals['msg'], with a fallback to plainproj.refine()if GSAS-II internals ever change — behavior can never be worse than before. Verified against the live failure case._code_hash.jsonregenerated. Note for review: kicker changes warrant the full regression suite on a real environment (pixi run test) before merge — refined-value tolerances especially.🤖 Generated with Claude Code