Add parameters needed for 6.3 lattice and magnet visualization. #890
Conversation
for more information, see https://pre-commit.ci
github-actions
bot
added
topic: far-forward
|
Hi, I have created a PR for reference, so everyone is aware of the upcoming updates to the IR: #893 |
It was a while ago I added that so not entirely sure. I believe the visualization wasn't working correctly without a detector element and the detector element didn't work without an id. Some/all of what has been added might be unnecessary. |
What was the problem with publishing the capybara here as is done in EICRecon? I didn't follow why it was added then reverted. |
There were errors in running it due to the different structure of other artifacts (different directory), so it always failed the pipelines. It wasn't immediately clear how to fix it, so we reverted to avoid blocking work. I haven't found the time to get back to that. #859 was supposed to fix it and includes a link to https://github.com/eic/epic/actions/runs/14817178151/job/41749142343#step:7:7 that shows how it failed. |
src/CylindricalMagnetChain_geo.cpp
Outdated
| DetElement yoke_de(sdet, name, id); | ||
| yoke_de.setPlacement(yoke_pv); | ||
| yoke_de.setAttributes(description, v_yoke, x_det.regionStr(), x_det.limitsStr(), vis_name); |
There was a problem hiding this comment.
I'd still like to avoid this, if we can. We only have 255 detector IDs in our 8-bit field for segmentations, and less than half are left. If we start giving IDs to all magnets, we're going to run out quickly.
Note that you can add regions and limits to volumes too:
DDCore/include/DD4hep/Volumes.h:
const Volume& setAttributes(const Detector& description, const std::string& region, const std::string& limits,
const std::string& vis) const;
Can you verify that visualization is not affected when avoiding the ID for this?
There was a problem hiding this comment.
I have removed the DetElement and recovered the visualization using setAttributes. The physical volume ID is still required to give the volume a unique identifier in the tree but shouldn't count towards the detector IDs
Briefly, what does this PR introduce?
Moves the code and visualization changes from #838 so that it will reflect the minimal differences required to update the lattice.
Mostly this adds parameters which are set to 0 in the current configuration but need to be set in the update.
What kind of change does this PR introduce?
Please check if this PR fulfills the following:
Does this PR introduce breaking changes? What changes might users need to make to their code?
No
Does this PR change default behavior?
No