Switch Shield

[eduard322]

Here is the stable version in which the usage of ROOT file for setting the dinensions of Muon Shield is removed (at least now it takes info directly from the geometry_config.py script). Several things to mention and to discuss:

1. During the simulation the geometry registration is conducted twice for some reason, which seems very redundant. First time in run_simScript.py and then in shipDet_conf.py. If there is no hidden reason, I can remove these entities.
2. Reading the Muon Shield dimensions from the dictionary in geometry_config.py script doesn't look perfect since the geometry of Muon Shield is not fixed yet. During optimization me and Evgeny were using json files for storing different geometries, would be nice to repeat this implementation now. Or maybe use yaml file since the info about decay volume is stored in yaml files and it would be more consistent.
3. Current warm version overlaps with the floor. Floor can be excavated though, we need a permission to make so, don't we?

[olantwin]

I'll need to do some testing, but some first comments that stood out while reading.

[olantwin]

1. During the simulation the geometry registration is conducted twice for some reason, which seems very redundant. First time in run_simScript.py and then in shipDet_conf.py. If there is no hidden reason, I can remove these entities.

What do you mean by registration?

2. Reading the Muon Shield dimensions from the dictionary in geometry_config.py script doesn't look perfect since the geometry of Muon Shield is not fixed yet. During optimization me and Evgeny were using json files for storing different geometries, would be nice to repeat this implementation now. Or maybe use yaml file since the info about decay volume is stored in yaml files and it would be more consistent.

I'm not sure what our UZH colleagues are doing now. A yaml would probably be nicest, as it allows us to add comments etc. and is both flexible and readable. Ideally, a format we could share with the SBT/vessel/other subsystems would be nice.

3. Current warm version overlaps with the floor. Floor can be excavated though, we need a permission to make so, don't we?

I believe the muon shield optimisation now includes constraints to avoid overlaps with the floor. Unfortunately, none are implemented yet. @Gfrisella, maybe you can provide the dimensions of a configuration we can use via the dictionary/yaml?

[eduard322]

The geometry is registered here https://github.com/eduard322/FairShip/blob/68fa73cede0025222016b4a05fd92f68a07f53cc/macro/run_simScript.py#L224
Then ship_geo goes to shipDet_conf.py script where another geometry registration is conducted https://github.com/eduard322/FairShip/blob/68fa73cede0025222016b4a05fd92f68a07f53cc/python/shipDet_conf.py#L151
with literally same parameters coming from the previous registration in run_simScript.py
Looks ambiguous

[olantwin]

The geometry is registered here https://github.com/eduard322/FairShip/blob/68fa73cede0025222016b4a05fd92f68a07f53cc/macro/run_simScript.py#L224 Then ship_geo goes to shipDet_conf.py script where another geometry registration is conducted https://github.com/eduard322/FairShip/blob/68fa73cede0025222016b4a05fd92f68a07f53cc/python/shipDet_conf.py#L151 with literally same parameters coming from the previous registration in run_simScript.py Looks ambiguous

Indeed, in the case of run_simScript.py it seems completely redundant, but in the case of loading old data for the reconstruction, it has its use for forward-compatibility.

Maybe we can find a way to avoid the unnecessary step in run_simScript.py.

[olantwin]

From my side this looks OK.

Please add a change log entry describing your change.

Since this affects the muon shield, I will wait for an OK from @Gfrisella on behalf of the muon shield before merging.

[olantwin]

pre-commit.ci run

[eduard322]

@olantwin sorry, stupid question: how is change log usually done? Should I add CHANGELOG.md to the commit or something?

[olantwin]

@olantwin sorry, stupid question: how is change log usually done? Should I add CHANGELOG.md to the commit or something?

It's just a text file. Add a succinct entry under unreleased describing the change, and commit the change to the pull request branch.

[olantwin]

The overlaps seem to be only the ones from #592 + #609 , no new overlaps are introduced 👍

[olantwin]

@Gfrisella Could you please review this pull request?

[olantwin]

Did some cleanup of the changelog and ran clang-format on the changed lines.

Looks good to me.

[olantwin]

@Gfrisella The muon shield group is welcome to suggest improvements. For now this seems like a clear step in the right direction.

@eduard322 Thanks to making a first step towards improving the muon shield implementation.