Skip to content
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

URLConfigs, Volume Plugins, S1 Plugin Split, Removal of electron_cloud data_kind #35

Merged
merged 37 commits into from
Jun 2, 2023

Conversation

HenningSE
Copy link
Collaborator

@HenningSE HenningSE commented May 10, 2023

This PR will:

  • replace all @strax.takes_config arguments by straxen.URLConfig
  • add a centralized context definition so the default values can be set there

This was referenced May 10, 2023
@HenningSE HenningSE marked this pull request as ready for review May 20, 2023 11:58
* Move S1 n_photon_hits calculation out of S1PhotonPropagation plugin in preparation for fastsim integration
@HenningSE HenningSE changed the title Update Config Handling URLConfigs, Volume Plugins, S1 Plugin Split May 20, 2023
@HenningSE HenningSE changed the title URLConfigs, Volume Plugins, S1 Plugin Split URLConfigs, Volume Plugins, S1 Plugin Split, Removal of electron_cloud data_kind May 22, 2023
@HenningSE HenningSE mentioned this pull request May 22, 2023
@HenningSE
Copy link
Collaborator Author

This PR grew a bit compared to the initial task. These things have been changed:

  • Use straxen.URLConfig instead of strax.takes_config
  • Define the simulation context and set all needed config values there. When fuse will be made public this part must be done somewhere else (cutax?)
  • Prepare fuse for the integration of fastsim by splitting the S1 simulation plugin and changing the S2 simulation so they can be loaded along with microphysics_summary and the s1_photons.
  • Remove epix detector module thing and replace it by VolumePlugins
  • Add a VerticalMergerPlugin to simplify the combination of some Plugins.

@HenningSE HenningSE requested a review from ramirezdiego May 22, 2023 14:02
@HenningSE HenningSE mentioned this pull request May 30, 2023
Copy link
Collaborator

@ramirezdiego ramirezdiego left a comment

Choose a reason for hiding this comment

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

After testing the notebook, I finally finished looking at all the code changes (there was a lot 😄), thanks for the patience.

To me this looks good and refines further the structure that we want, to deal with detector corrections and fast-sim implementation, also thanks to the extra couple of general plugins that you added. This is great work, @HenningSE :) !

Let's deal with the TOC and renaming issues in the next iterations, but merge all these changes now, so that we can already start working on top of the new structure.

@ramirezdiego ramirezdiego merged commit 4d82db4 into main Jun 2, 2023
@ramirezdiego ramirezdiego deleted the config_change branch June 2, 2023 16:28
@ramirezdiego ramirezdiego mentioned this pull request Jun 29, 2023
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.

2 participants