Skip to content

fix broken initialization of PolarizedQProbe #344

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bmaranville merged 3 commits into from
Nov 14, 2025
Merged

fix broken initialization of PolarizedQProbe #344

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c172daa
fix broken initialization of PolarizedQProbe
bmaranville Oct 31, 2025
ce430e6
delegate key generation to xs class in _calculate_union, to check for…
bmaranville Nov 13, 2025
23c225b
adding test for initializing PolarizedQProbe
bmaranville Nov 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion refl1d/probe/probe.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1636,7 +1636,7 @@ def oversample(self, n=20, seed=1):
oversample.__doc__ = Probe.oversample.__doc__

def _calculate_union(self):
theta_offsets = [x.theta_offset.value for x in self.xs if x is not None]
theta_offsets = [x.theta_offset.value for x in self.xs if x is not None and hasattr(x, "theta_offset")]
Copy link
Member

@pkienzle pkienzle Nov 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The measurement union function uses both Q (hence the dependence on theta_offset) and also dQ, therefore sample_broadening should also be part of the key.

Rather than assuming these properties exist and that they are the only properties that affect the measurement union, we could be delegating to the underlying class, with something like a xs._QdQ_key() method. This will matter more when we have full support for ProbeSet as polarized cross sections.

Copy link
Member Author

@bmaranville bmaranville Nov 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good catch on the sample_broadening dependency... this optimization (_calculate_union) is pretty useful in reducing calculation time but it adds a lot of complexity!

union_cache_key = (theta_offsets, self.oversampling, self.oversampling_seed, tuple(self.oversampled_regions))
if self._union_cache_key == union_cache_key:
# no change in offsets or oversampling: use cached values of measurement union
Expand Down Expand Up @@ -1929,6 +1929,7 @@ def __init__(
self.oversampling_seed = oversampling_seed
self.oversampled_regions = oversampled_regions if oversampled_regions is not None else []
# this sets self.Q, self.dQ, self._calc_Q:
self._union_cache_key = None
self._calculate_union()

@property
Expand Down
Loading