Switch to a more sensible normalization strategy for loss metrics#70
Open
Switch to a more sensible normalization strategy for loss metrics#70
Conversation
…y which allows for a nice normalization of the Poisson NLL. The slow tests will now likely fail
…malizations, and add the new normalization to all models
Adds 'intensity_mse' as a selectable loss function (alongside the existing 'amplitude_mse' and 'poisson_nll') to FancyPtycho, MultislicePtycho, Bragg2DPtycho, Multislice2DPtycho, and RPI. Models that previously had a hardcoded amplitude_mse assignment now use the same configurable pattern as FancyPtycho, with the loss parameter threaded through from_dataset and from_calibration as well. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Updates tutorial_simple_ptycho.py to match simple_ptycho.py: removes the def loss() method and instead assigns self.loss and self.loss_normalizer as instance attributes in __init__. Updates tutorial.rst accordingly: adds the loss assignment to the __init__ code block with an explanation, and removes def loss() from the forward model section. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
poisson_nll now returns a sum rather than a mean, consistent with the normalizer pattern. Remove the per-pixel divisions from the numpy reference calculations accordingly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Use poisson_nll in test_near_field_ptycho and intensity_mse in test_Adam_gold_balls to exercise these loss paths end-to-end. Thresholds left as-is pending re-running on a GPU machine. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Revert test_Adam_gold_balls to default amplitude_mse - Add new test_intensity_MSE test (gold balls + AdamReconstructor) - Update near_field threshold to 17 (poisson_nll scale) Thresholds for test_intensity_MSE to be tuned after GPU run. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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.
This makes loss metrics a bit more interpretable. I may put some more work into writing better normalizations if I have a similar internet-free train ride in the future, where I can do some cdtools work but don't have a stable connection to any GPUS...
Key notes:
Changes: