Conversation
github-actions
bot
added
topic: forward
|
@locchiuto Are you able to address the review comments? Let me know if something is unclear. @chchatte92 Did you have a chance to look at this? |
|
|
@locchiuto are these comments clear to you and can you react to the comments? |
locchiuto
force-pushed
the
dRICH_tiled_aerogel_Luisa
branch
from
d2ccef3 to
8c546d4
Compare
for more information, see https://pre-commit.ci
compact/pid/drich.xml
Outdated
| num="DRICH_num_coronas" | ||
| material="CarbonFiber_15percent" | ||
| vis="DRICH_filter_vis" | ||
| d= "DRICH_d_square" |
There was a problem hiding this comment.
Please provide a meaningful name to this variable.
DRICH_d_square can be like DRICH_aero_max_size
d can be like size_max.
Ensure that no name conflicts.
There was a problem hiding this comment.
@chchatte92 The DRICH_aero_max_size is your suggestion for the name?
There was a problem hiding this comment.
Something like that. But this Luisa should decide.
There was a problem hiding this comment.
Something like that. But this Luisa should decide.
There was a problem hiding this comment.
This value is not even accessed, should we remove it for now?
|
@locchiuto did you push your changes to your branch? |
Please ignore. I can see the changes. |
3 participants
Briefly, what does this PR introduce?
The aerogel, previously implemented as a single block, has been divided into two perfectly overlapping parts. A carbon-fiber structure has been integrated within the configuration, designed to mechanically support and encapsulate each aerogel element.
The main structural parameters are now easily configurable through the drich.xml file located in epic/compact/pid, without requiring direct modifications to the source code.
What kind of change does this PR introduce?
Tiled aerogel optics, tunable size
Please check if this PR fulfills the following:
Does this PR introduce breaking changes? What changes might users need to make to their code?
Does this PR change default behavior?