EQSANS add Blocked Beam #1066
EQSANS add Blocked Beam #1066
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## next #1066 +/- ##
==========================================
- Coverage 85.89% 85.80% -0.09%
==========================================
Files 101 101
Lines 11334 11356 +22
==========================================
+ Hits 9735 9744 +9
- Misses 1599 1612 +13 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for blocked beam run normalization in EQSANS data reduction. The blocked beam measurement allows for proper background correction when dark current files are not available.
Key changes:
- Added
blockedBeamRunNumberconfiguration parameter with mutual exclusivity validation againstdarkFileName - Implemented blocked beam workspace loading, processing, and subtraction from sample and background data
- Enhanced normalization to log when using proton charge vs other duration methods for blocked beam cases
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/drtsans/configuration/schema/EQSANS.json | Added blockedBeamRunNumber schema definition with validation ensuring mutual exclusivity with darkFileName |
| src/drtsans/tof/eqsans/api.py | Added blocked beam loading, processing logic, and subtraction from sample/background workspaces |
| src/drtsans/tof/eqsans/reduction_api.py | Added has_blocked_beam parameter to prepare_data_workspaces and workspace cleanup for blocked beam runs |
| src/drtsans/tof/eqsans/normalization.py | Added logging for blocked beam normalization and passed has_blocked_beam flag through normalization chain |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if blocked_beam.data: | ||
| bkgd_ws = subtract_background(bkgd_ws, blocked_ws) |
There was a problem hiding this comment.
The variable blocked_ws may not be defined at line 611 if the condition at line 608 (not registered_workspace(bkgd_ws_name)) is True but the condition at line 581 (not registered_workspace(blocked_ws_name)) is False. In this scenario, blocked_ws is retrieved from mtd at line 584 but only within the scope of the if-block starting at line 579. This would cause a NameError. The check should be if blocked_beam.data and blocked_beam.data is not None: and blocked_ws should be retrieved outside the inner if-block or the entire blocked beam processing logic should be restructured to ensure blocked_ws is always available when needed.
| if blocked_beam.data and blocked_ws: | ||
| blocked_ws.delete() |
There was a problem hiding this comment.
The variable blocked_ws may not be defined at line 635. If blocked_beam.data exists but the workspace was already registered (line 581-584 evaluated with the else branch), blocked_ws is only defined within the scope of the if-block starting at line 579. This will cause a NameError when attempting to delete. The blocked_ws variable should be defined in a broader scope or the deletion logic should be moved inside the block where blocked_ws is guaranteed to exist.
| if duration.log_key == "proton_charge": | ||
| logger.notice("Workspace normalized by Time using proton charge duration.") | ||
| else: | ||
| logger.warning(f"Workspace normalized by Time using {duration.log_key}.") |
There was a problem hiding this comment.
The warning message is unclear about why this is a warning vs. a notice. If using a non-proton-charge duration for blocked beam runs is problematic, the message should explain the potential issue or provide guidance. Consider clarifying why this situation warrants a warning, e.g., 'Workspace normalized by Time using {duration.log_key} instead of recommended proton_charge for blocked beam runs.'
| logger.warning(f"Workspace normalized by Time using {duration.log_key}.") | |
| logger.warning( | |
| f"Workspace normalized by Time using {duration.log_key} instead of recommended 'proton_charge' for blocked beam runs. " | |
| "This may lead to incorrect normalization. If possible, use 'proton_charge' for normalization of blocked beam runs." | |
| ) |
jmborr
left a comment
jmborr
left a comment
There was a problem hiding this comment.
The treatment of the block beam is incorrect. As I understand it, it should mimic the treatment for the dark current. We should have a meeting with Yingrui and one of the EQSANS IS to go over this.
| "examples": ["Monitor", "Total charge", "Time"], | ||
| "default": "Total charge" | ||
| }, | ||
| "blockedBeamRunNumber": { |
There was a problem hiding this comment.
use validator onlyOneTrue instead of evaluateCondition. See options useLogSlice and useTimeSlice in file common.json for an example usage.
Meaning of validator onlyOneTrue is explained in docstring of function _validate_only_one_true()
|
|
||
| # blocked beam | ||
| # (run_type, run_number, is_elastic_reference) | ||
| other_runs = [ |
There was a problem hiding this comment.
a superior choice to the infamous index scheme 😁
| sample_transmission=ws_mon_pair(data=sample_transmission_ws, monitor=sample_transmission_mon_ws), | ||
| background_transmission=ws_mon_pair(data=background_transmission_ws, monitor=background_transmission_mon_ws), | ||
| dark_current=ws_mon_pair(data=dark_current_ws, monitor=dark_current_mon_ws), | ||
| dark_current=ws_mon_pair(dark_current_ws, None), |
There was a problem hiding this comment.
| dark_current=ws_mon_pair(dark_current_ws, None), | |
| dark_current=ws_mon_pair(dark_current_ws, monitor=None), # dark current requires no monitor data |
| background_transmission=ws_mon_pair(data=background_transmission_ws, monitor=background_transmission_mon_ws), | ||
| dark_current=ws_mon_pair(data=dark_current_ws, monitor=dark_current_mon_ws), | ||
| dark_current=ws_mon_pair(dark_current_ws, None), | ||
| blocked_beam=ws_mon_pair(blocked_beam_ws, None), |
There was a problem hiding this comment.
| blocked_beam=ws_mon_pair(blocked_beam_ws, None), | |
| blocked_beam=ws_mon_pair(blocked_beam_ws, monitor=None), # blocked beam requires no monitor data |
| if not registered_workspace(ws_name): | ||
| dark_current_file = abspath(dark_current_file) | ||
| print(f"Loading filename {dark_current_file}") | ||
| print(f"Loading dark current {dark_current_file}") |
There was a problem hiding this comment.
| print(f"Loading dark current {dark_current_file}") | |
| logger.info(f"Loading dark current {dark_current_file}") |
| ("blocked_beam", blocked_beam_run, False), | ||
| ] | ||
|
|
||
| other_ws_dict = {} |
There was a problem hiding this comment.
| other_ws_dict = {} | |
| other_ws_dict = {} # stores references to workspaces other than the sample |
| "mask_btp": mask_btp, | ||
| "solid_angle": solid_angle, | ||
| "sensitivity_workspace": sensitivity_workspace, | ||
| "has_blocked_beam": (blocked_beam is not None and blocked_beam.data is not None), |
There was a problem hiding this comment.
I've looked at the effect of key "has_blocked_beam". Apparently it has none! I went down the rabbit hole:
prepare_data_workspaces(has_blocked_beam)
-->normalize_by_flux(has_blocked_beam)
-->normalize_by_time(has_blocked_beam)
And then normalize_by_time logs a message depending on the value of has_blocked_beam. It's not worth it modifying the signature of so many functions just for the sake of one log message
| # process sample | ||
| sample_ws = prepare_data_workspaces(sample_ws_raw, output_workspace=output_workspace, **prepare_data_conf) | ||
|
|
||
| if blocked_beam is not None and blocked_beam.data is not None: |
There was a problem hiding this comment.
the blocked beam run does not require prepare_data_workspaces to be applied to it.
| bkgd_ws_name = output_suffix + "_background" | ||
| if not registered_workspace(bkgd_ws_name): | ||
| bkgd_ws = prepare_data_workspaces(bkg_ws_raw, output_workspace=bkgd_ws_name, **prepare_data_conf) | ||
| if blocked_beam.data: |
There was a problem hiding this comment.
subtracting the blocked beam from a run should be part of prepare_data_workspaces. In this function:
if dark_current is not None and dark_current.data is not None:
subtract_dark_current(output_workspace, dark_current.data)
elif blocked_beam is not None and blocked_beam.data is not None:
subtract_blocked_beam(output_workspace, dark_current.data)
and a dedicated subtract_blocked_beam() should be created. This function should be a clone of subtract_dark_current and the blocked beam run subject to the same normalization as the dark current.
Description of work:
This adds logic to EQSANS so that the blocked beam can be processed. Either the
blockedBeamRunNumberordarkFileNamecan be specified but not both.Workflow overview
Check all that apply:
References:
The following configuration can be used to test an EQSANS reduction with blocked beam.
{ "schemaStamp": "2025-10-23T04:02:08.634003", "instrumentName": "EQSANS", "iptsNumber": "36254", "sample": { "runNumber": "167860", "loadOptions": null, "thickness": 1, "transmission": { "runNumber": "167855", "value": "" } }, "background": { "runNumber": "167859", "transmission": { "runNumber": "167854", "value": "" } }, "emptyTransmission": { "runNumber": "167853", "value": null }, "beamCenter": { "runNumber": "167853" }, "outputFileName": "porsil_final_conf1", "configuration": { "outputDir": "", "instrumentConfigurationDir": "/SNS/EQSANS/shared/instrument_configuration", "useTimeSlice": false, "timeSliceInterval": 300, "timeSliceOffset": 0.0, "timeSlicePeriod": null, "useLogSlice": false, "logSliceName": null, "logSliceInterval": 10, "cutTOFmin": 1000, "cutTOFmax": 2000, "wavelengthStep": 0.1, "wavelengthStepType": "constant Delta lambda", "sampleOffset": 285, "useDetectorOffset": true, "detectorOffset": 60.999582, "sampleApertureSize": 10, "sourceApertureDiameter": null, "usePixelCalibration": null, "maskFileName": "/SNS/EQSANS/IPTS-36254/shared/gn/maskWS4m10A_10mm.nxs", "useDefaultMask": null, "defaultMask": null, "useMaskBackTubes": null, "normalization": "Total charge", "fluxMonitorRatioFile": null, "beamFluxFileName": "/SNS/EQSANS/shared/NeXusFiles/EQSANS/2025B_mp/bl6_flux_2025B_Aug_rebinned_4m.txt", "sensitivityFileName": "/SNS/EQSANS/shared/NeXusFiles/EQSANS/2025B_mp/Sensitivity_patched_thinPMMA_4m_167517.nxs", "useSolidAngleCorrection": true, "useThetaDepTransCorrection": true, "mmRadiusForTransmission": 25, "absoluteScaleMethod": null, "StandardAbsoluteScale": 1.72, "numQxQyBins": 80, "1DQbinType": "scalar", "QbinType": "log", "numQBins": 100, "LogQBinsPerDecade": null, "useLogQBinsDecadeCenter": false, "useLogQBinsEvenDecade": false, "WedgeMinAngles": "-30, 60", "WedgeMaxAngles": "30, 120", "autoWedgeQmin": null, "autoWedgeQmax": null, "autoWedgeQdelta": null, "autoWedgeAzimuthalDelta": null, "autoWedgePeakWidth": null, "autoWedgeBackgroundWidth": null, "autoWedgeSignalToNoiseMin": null, "autoWedgePhiMin": "", "autoWedgePhiMax": "", "AnnularAngleBin": 5.0, "Qmin": null, "Qmax": null, "useErrorWeighting": false, "smearingPixelSizeX": null, "smearingPixelSizeY": null, "useSubpixels": null, "subpixelsX": null, "subpixelsY": null, "useSliceIDxAsSuffix": null, "fitInelasticIncoh": false, "selectMinIncoh": false, "incohfit_qmin": 0.08, "incohfit_qmax": 0.09, "incohfit_factor": null, "outputWavelengthDependentProfile": true, "incohfit_intensityweighted": false, "elasticReference": { "runNumber": "", "thickness": "1.0", "transmission": { "runNumber": null, "value": "0.9" } }, "elasticReferenceBkgd": { "runNumber": "", "transmission": { "runNumber": null, "value": "0.9" } }, "scaleComponents": { "detector1": [ 1.021, 1.0204066565, 1 ] }, "blockedBeamRunNumber": 167862, "darkFileName": null, "autoSymmetricWedges": "", "removeAlgorithmHistory": false }, "dataDirectories": "" }Check list for the reviewer
Execution of tests requiring the /SNS and /HFIR filesystems
It is strongly encouraged that the reviewer runs the following tests in their local machine
because these tests are not run by the GitLab CI. It is assumed that the reviewer has the /SNS and /HFIR filesystems
remotely mounted in their machine.
In the above code snippet, substitute
<MERGE_REQUEST_NUMBER>for the actual merge request number. Also substitute<my_drtsans_dev_environment>with the name of the conda environment you use for development. It is critical thatyou have installed the repo in this conda environment in editable mode with
pip install -e .orconda develop .