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

make material in 1st layer of ECAL absorber configurable #407

Conversation

giovannimarchiori
Copy link
Contributor

@giovannimarchiori giovannimarchiori commented Nov 4, 2024

Make material in 1st layer of noble-liquid ECAL absorber configurable.
Set by default to G10 rather than LAr.
Also add more comments and fix some whitespaces

BEGINRELEASENOTES

  • Make material in 1st layer of noble-liquid ECAL absorber configurable. Set by default to G10 rather than LAr.
    ENDRELEASENOTES

Here are a couple of plots that compare cellID and energy with the previous code and with the new code when the material in 1st layer is set to LAr via xml, to confirm that the code reshuffling itself has no impact on physics

ECalBarrelModuleThetaMergedPositioned cellID_comparison_old_vs_new
ECalBarrelModuleThetaMergedPositioned energy_comparison_old_vs_new

…ult to G10 rather than LAr. Also add more comments and fix some whitespaces
… by default to G10 rather than LAr. Also add more comments and fix some whitespaces"

This reverts commit 33615be.
@giovannimarchiori
Copy link
Contributor Author

@BrieucF has we agreed, I moved the changes to ALLEGRO_o1_v04

@BrieucF
Copy link
Contributor

BrieucF commented Nov 8, 2024

Thanks Giovanni! You prefer to have the change to have all layers with the same "radial" extent in another PR?

@giovannimarchiori
Copy link
Contributor Author

Yes let's reserve that for another PR (are we sure we want to have both changes applied in the same detector model)?

@BrieucF
Copy link
Contributor

BrieucF commented Nov 8, 2024

are we sure we want to have both changes applied in the same detector model

I'd say yes, because we bump the version we can apply significant changes. And if we decide that the default in v04 is to have pre-sampler with lead, then it goes int he direction of having the first layer as a normal layer. But we can re-discuss later.

@BrieucF BrieucF merged commit 4a90833 into key4hep:main Nov 8, 2024
5 of 6 checks 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.

2 participants