Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Comments

Close side panel

Adding Preshower detector (PSR) for ALICE3 simulation#6519

Closed
an15ms157 wants to merge 18 commits intoAliceO2Group:devAliceO2Group/AliceO2:devfrom
an15ms157:devan15ms157/AliceO2:devCopy head branch name to clipboard
Closed

Adding Preshower detector (PSR) for ALICE3 simulation#6519
an15ms157 wants to merge 18 commits intoAliceO2Group:devAliceO2Group/AliceO2:devfrom
an15ms157:devan15ms157/AliceO2:devCopy head branch name to clipboard

Conversation

@an15ms157
Copy link

This PR creates a simple 8 layered preshower detector named PSR. This is a cylindrical detector of length 100 cm whose shower layers are made up of Pb (0.5 cm) and the detector layers are made up of Si (45 microns). PSR is based on the ITSMFT classes and the code is structurally similar to that of FT3. Each layer is made of a monolithic silicon disk with a thin sensitive layer for hit generation. Silicon chip thickness is tuned to match the layer x/X0 to allow a minimal evaluation of material budget effects.

One should get a file o2sim_HitsPSR.root by running
$ o2-sim -m PSR -e TGeant3 -g boxgen -n 10

@mconcas
Copy link
Collaborator

mconcas commented Jul 9, 2021

ok, now that codechecker is fine the next step is to reabase your branch on the AliceO2/dev one. Since the pragma cheker is something that has been added in relatively recent times wrt your PR opening.

@an15ms157
Copy link
Author

I have made the changes.

Copy link
Collaborator

@mconcas mconcas left a comment

Choose a reason for hiding this comment

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

To me looks pretty good for the part of the upgrades, some inline comments.

// inserts the last
void MaterialManager::insertTGeoMedium(std::string modname, int localindex)
{
LOG(INFO) << "Changes for build";
Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps a DEBUG print you forgot?

Copy link
Author

Choose a reason for hiding this comment

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

Ah!! yes

mLayerID.clear();
mLayers.resize(2);

for (Int_t direction : {0, 1}) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is not really clear to me, why these changes in FT3 whould affact the PSR? @rpezzi is it fine to you?

\page refDetectorsUpgradesALICE3FT3 EndCaps
/doxy -->

# PostLS4EndCaps
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is minor, but I think should be changed to desccribe PSR

@mconcas
Copy link
Collaborator

mconcas commented Jul 14, 2021

@an15ms157 : do you have any update on this? Could you also rebase on dev branch, so that the git history is linear? Thanks

@mconcas
Copy link
Collaborator

mconcas commented Jul 22, 2021

Hi @an15ms157, you should rebase the modifications you want to merge on the dev branch. The history must be linear, e.g. no merging commits and branching. Could you please do that?

@an15ms157
Copy link
Author

Hello @mconcas
I am not sure that I have as of yet managed to make the required changes. So, I have tried to make a new local branch, clump all the content to a single commit and push it to my fork. Here is it: https://github.com/an15ms157/AliceO2/tree/mydev
Is it possible that I run another PR with this different branch? In case if such is possible and it is sucessful, I will keep that one and remove this PR.

@mconcas
Copy link
Collaborator

mconcas commented Jul 23, 2021

Hello @mconcas
I am not sure that I have as of yet managed to make the required changes. So, I have tried to make a new local branch, clump all the content to a single commit and push it to my fork. Here is it: https://github.com/an15ms157/AliceO2/tree/mydev
Is it possible that I run another PR with this different branch? In case if such is possible and it is sucessful, I will keep that one and remove this PR.

Hi @an15ms157: I had a look to your new branch and it seems to be ok, please proceed with a new PR. Thanks.

@an15ms157
Copy link
Author

an15ms157 commented Jul 23, 2021

Hello @mconcas
I am not sure that I have as of yet managed to make the required changes. So, I have tried to make a new local branch, clump all the content to a single commit and push it to my fork. Here is it: https://github.com/an15ms157/AliceO2/tree/mydev
Is it possible that I run another PR with this different branch? In case if such is possible and it is sucessful, I will keep that one and remove this PR.

Hi @an15ms157: I had a look to your new branch and it seems to be ok, please proceed with a new PR. Thanks.
Hello @mconcas .Here is a link to my new PR: #6727

@mconcas mconcas closed this Jul 23, 2021
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.

3 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.