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

Reintroduce Wrapper Volume 0 for IB Layers of ITS3 Upgrade #5441

Merged
merged 1 commit into from Feb 11, 2021

Conversation

mario6829
Copy link
Contributor

@mario6829 mario6829 commented Feb 10, 2021

To make the development of Upgraded ITS Inner Barrel simpler, the Wrapper Volume 0 holding the IB was removed. Since now new IB volumes are planned to be implemented, in order to keep well separated the IB and make the geometry tree cleaner, the Wrapper Volume 0 has been put back in place. Its radial extension is not fixed but based on the actual IB extension, so in case the radii of IB Layers are changed, the Wrapper Volume adapts automatically.
This modification is preparatory to the implementation of new ITS3 Inner Barrel elements for further studies.

@mario6829
Copy link
Contributor Author

Tagging @mconcas to follow this PR.


// Redefine Wrapper Volume 0 using information on IB radii
static constexpr float safety = 0.5; // Educated guess, may be improved
its->defineWrapperVolume(0, tdr5data[0][0] - safety, tdr5data[3][0] + safety, wrpZSpan[0]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

@mario6829 does not this repeat the volume for iw=0 in

for (int iw = 0; iw < kNWrapVol; iw++) {
its->defineWrapperVolume(iw, wrpRMin[iw], wrpRMax[iw], wrpZSpan[iw]);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @shahor02
as the comment says, it is done on purpose because only at this stage of the code the IB radii are known (they are defined at lines 149-153). I preferred not to fix the wrapper radii but compute them from innermost and outermost IB cylinders in order to leave the possibility of changing the IB radii - in case this is needed for further studies - without running the risk of extrusions or worrying about changing other lines.
But if you think it is cleaner, I can define only wrappers 1 and 2 in lines 124-125, or I can move altogether the definition of all wrappers in lines 166-167. Please let me know your suggestions.
Many thanks. Cheers
Mario

Copy link
Collaborator

Choose a reason for hiding this comment

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

@mario6829 OK, sorry, I did not realize that this is only to book the parameters and the actual volume additions follows later.

@mconcas
Copy link
Collaborator

mconcas commented Feb 10, 2021

Hi @mario6829 , thanks for the PR.
True, it looks like the construction was invoked already.

(Don't mind my approval, I was just checking if the CODEOWNERSHIP works, as I have been put in the file. Sadly it does not @qgp ).


// Redefine Wrapper Volume 0 using information on IB radii
static constexpr float safety = 0.5; // Educated guess, may be improved
its->defineWrapperVolume(0, tdr5data[0][0] - safety, tdr5data[3][0] + safety, wrpZSpan[0]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

@mario6829 OK, sorry, I did not realize that this is only to book the parameters and the actual volume additions follows later.

@shahor02
Copy link
Collaborator

failure unrelated

@shahor02 shahor02 merged commit 7301461 into AliceO2Group:dev Feb 11, 2021
@mario6829 mario6829 deleted the its3geo branch February 11, 2021 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants