Skip to content

Added initial foil analysis - #5

Merged
RemDelaporteMathurin merged 40 commits into
LIBRA-project:mainfrom
cdunn314:collin_foils
Jul 23, 2025
Merged

Added initial foil analysis#5
RemDelaporteMathurin merged 40 commits into
LIBRA-project:mainfrom
cdunn314:collin_foils

Conversation

@cdunn314

@cdunn314 cdunn314 commented Jul 2, 2025

Copy link
Copy Markdown
Contributor

Added code using libra-toolbox to process foil measurements. This is a work in progress, and I'd like feedback on how to improve the notebook and better integrate it into the rest of the data processing. Then we can apply a similar approach to the other runs.

@RemDelaporteMathurin RemDelaporteMathurin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think the overall workflow is sound. Maybe a few improvements would be to add some text to guide readers or headings (eg. Efficiency curve calculation, etc)

Let's also avoid dead code (ie. large code blocks commented out)

Comment thread analysis/neutron/foil_analysis.ipynb
Comment thread analysis/neutron/foil_analysis.ipynb
Comment thread analysis/neutron/foil_analysis.ipynb
Comment thread analysis/neutron/foil_analysis.ipynb
Comment thread analysis/neutron/foil_analysis.ipynb
Comment thread data/processed_data.json
Comment thread analysis/neutron/process_foil_data.py Outdated
@cdunn314

Copy link
Copy Markdown
Contributor Author

I am trying to get the reading from spectra from a saved h5 file to work, but some necessary functions like compute_detection_efficiency on a CheckSourceMeasurement class require there to be events, so looks like if we want to implement this with the h5 spectra reader, we'll need to change libra-toolbox

@cdunn314

Copy link
Copy Markdown
Contributor Author

I am trying to get the reading from spectra from a saved h5 file to work, but some necessary functions like compute_detection_efficiency on a CheckSourceMeasurement class require there to be events, so looks like if we want to implement this with the h5 spectra reader, we'll need to change libra-toolbox

This was not the case, I was mistaken

@cdunn314

Copy link
Copy Markdown
Contributor Author

@RemDelaporteMathurin do we want to add tests in this PR or a separate one?

@cdunn314

Copy link
Copy Markdown
Contributor Author

@RemDelaporteMathurin do we want to add tests in this PR or a separate one?

I realized that the run repos don't have pytests, just continuous integration, so I added running the foils_analysis.ipynb notebook to the CI.

Comment thread analysis/neutron/process_foil_data.py Outdated

@RemDelaporteMathurin RemDelaporteMathurin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just a bunch of final comments and then we can merge this!

@veliki-filozof feel free to have a look too!

Comment thread analysis/neutron/process_foil_data.py Outdated
Comment thread analysis/neutron/process_foil_data.py Outdated
Comment thread analysis/neutron/process_foil_data.py Outdated
Comment thread analysis/neutron/process_foil_data.py Outdated
Comment thread data/processed_data.json
Comment thread .github/workflows/ci.yml
Comment thread environment.yml Outdated
@veliki-filozof

Copy link
Copy Markdown
Collaborator

Just a bunch of final comments and then we can merge this!

@veliki-filozof feel free to have a look too!

Looks great to me, thanks @cdunn314 for working this out! I didn't review the detailed mechanics of how you process the data but the broad logic looks sound to me.

Only thing I'd like to check for easier integration in the following runs: What files should be copied verbatim, and in which files is there run-specific data that needs to be manually tweaked? Perhaps this could be added to the readme checklist too

Comment thread environment.yml Outdated
@cdunn314

Copy link
Copy Markdown
Contributor Author

I added papermills to the environment.yml file so that no matter what we set the download_from_raw variable to in the notebook when commits are pushed, it will be overwritten to False during the CI and processing: 0722526

@RemDelaporteMathurin

Copy link
Copy Markdown
Member

I added papermills to the environment.yml file so that no matter what we set the download_from_raw variable to in the notebook when commits are pushed, it will be overwritten to False during the CI and processing: 0722526

Nice find!

@cdunn314

Copy link
Copy Markdown
Contributor Author

I'm finished with this batch of changes. @RemDelaporteMathurin @veliki-filozof if everything looks good, I think we can merge this.

@RemDelaporteMathurin RemDelaporteMathurin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @cdunn314 happy to merge this! I like the attenuation factor interpolation function, I think this could go in libra-toolbox also!

@RemDelaporteMathurin
RemDelaporteMathurin merged commit c781e3f into LIBRA-project:main Jul 23, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants