-
Notifications
You must be signed in to change notification settings - Fork 2
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.
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
Support event-mode I(Q) #65
Changes from 5 commits
c17c64d
be91239
c0c105b
54dfa06
2f82e33
0b4880a
ca4eb37
5439933
cb97c6e
50122c3
6268fbe
2a728d7
146ac86
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,6 +21,7 @@ | |
) | ||
from .common import transmission_from_background_run, transmission_from_sample_run | ||
from .direct_beam import direct_beam | ||
from .types import BackgroundSubtractedIofQ, IofQ, ReturnEvents, SampleRun | ||
|
||
providers = ( | ||
*conversions.providers, | ||
|
@@ -41,6 +42,7 @@ | |
del importlib | ||
|
||
__all__ = [ | ||
SimonHeybrock marked this conversation as resolved.
Show resolved
Hide resolved
|
||
'ReturnEvents', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Above, you import There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed! |
||
'beam_center_finder', | ||
'common', | ||
'conversions', | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -51,3 +51,32 @@ def _no_variance_broadcast( | |
return (data.variances is None) or all( | ||
data.sizes.get(dim) == size for dim, size in sizes.items() | ||
) | ||
|
||
|
||
def broadcast_to_events_with_upper_bound_variances( | ||
da: sc.DataArray, *, events: sc.DataArray | ||
) -> sc.DataArray: | ||
""" | ||
Upper-bound estimate for errors from normalization in event-mode. | ||
|
||
Count the number of events in each bin of the input data array. Then scale the | ||
variances by the number of events in each bin. An explicit broadcast is performed | ||
to bypass Scipp's safety check on broadcasting variances. | ||
|
||
Details will be published in an upcoming publication by Simon Heybrock et al. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would be careful with making such promises about publishing something. This may never happen? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Then we should not use it. Anyway, I have a very early draft, but it needs more work. |
||
""" | ||
if da.variances is None: | ||
return da | ||
constituents = events.bins.constituents | ||
Q = constituents['data'].coords['Q'] | ||
constituents['data'] = sc.DataArray( | ||
sc.ones(sizes=Q.sizes, dtype='float32'), coords={'Q': Q} | ||
) | ||
# Combine all bins of the events that correspond to the same bin in the input data | ||
to_concat = set(events.dims) - set(da.dims) | ||
binned = sc.DataArray(sc.bins(**constituents).bins.concat(to_concat)) | ||
counts = binned.hist(Q=da.coords['Q']) | ||
da = da.copy() | ||
da.variances *= counts.values | ||
da.data = sc.bins_like(events, da.data) | ||
return da |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure why we are now importing those on the top level? Was it to get away from the
from types import *
?If so, should this be reflected in the notebooks?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes.
types
is meaningless for the user. We should name (and categorize things) in a meaningful way. Yes, there are a lot of things that need updating, I wanted to do that separately, in particular given that there is also other ongoing work that may conflict.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we sit down and discuss what would be the best way to do this, so we're all on the same page?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Definitely!