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

dRICH large sensor for focal point region studies #351

Open
wants to merge 24 commits into
base: main
Choose a base branch
from

Conversation

cpecar
Copy link

@cpecar cpecar commented Jan 13, 2023

Briefly, what does this PR introduce?

Adds two new optics debugging mode for dRICH:
4 - creates large photosensor instead of default dRICH sensors, designed to catch all possible reflected photons. This is currently used with test 12 / 14 in drich-devs, throwing parallel beams of photons in order to determine the focal region of the dRICH optics.
5 - from a local text file, creates representations of the calculated focal points from mode 4 and a script in drich-devs, and also creates default dRICH sensors to visually compare calculated focal point to sensor position.

What kind of change does this PR introduce?

  • Bug fix (issue #__)
  • New feature (issue #__)
  • Documentation update
  • Other: __

Please check if this PR fulfills the following:

  • Tests for the changes have been added
  • Documentation has been added / updated
  • Changes have been communicated to collaborators

Does this PR introduce breaking changes? What changes might users need to make to their code?

No.

Does this PR change default behavior?

No.

… studies of focal point positions). Also added option to draw calcualted focal points from local text file (generated with script in drich-dev)
…tor of dRICH, and one to draw calculated focal points alongside normal sensors tiled on sphere
@cpecar cpecar requested a review from c-dilks January 13, 2023 18:43
@github-actions github-actions bot added topic: forward Positive-rapidity detectors (hadron-going side) topic: PID Particle identification labels Jan 13, 2023
@c-dilks
Copy link
Member

c-dilks commented Jan 13, 2023

Looks good to me. Where should we store your code to calculate the focal points? Here or in drich-dev?

@c-dilks c-dilks requested a review from wdconinc January 13, 2023 20:16
@cpecar
Copy link
Author

cpecar commented Jan 13, 2023

The focal point determination script is already in drich-dev (PR #55 in drich-dev), so I would say there makes more sense.

c-dilks
c-dilks previously approved these changes Jan 13, 2023
compact/pid/drich.xml Outdated Show resolved Hide resolved
src/DRICH_geo.cpp Outdated Show resolved Hide resolved
src/DRICH_geo.cpp Outdated Show resolved Hide resolved
src/DRICH_geo.cpp Outdated Show resolved Hide resolved
src/DRICH_geo.cpp Outdated Show resolved Hide resolved
src/DRICH_geo.cpp Outdated Show resolved Hide resolved
src/DRICH_geo.cpp Outdated Show resolved Hide resolved
src/DRICH_geo.cpp Outdated Show resolved Hide resolved
src/DRICH_geo.cpp Outdated Show resolved Hide resolved
src/DRICH_geo.cpp Outdated Show resolved Hide resolved
@c-dilks
Copy link
Member

c-dilks commented Jan 25, 2023

@cpecar I updated this branch w.r.t. main and resolved conflicts (hopefully correctly).

@cpecar cpecar requested a review from c-dilks February 8, 2023 17:24
@cpecar cpecar requested a review from wdconinc February 8, 2023 17:24
src/DRICH_geo.cpp Outdated Show resolved Hide resolved
src/DRICH_geo.cpp Outdated Show resolved Hide resolved
case 5:
vesselSolid = vesselBox;
gasvolSolid = gasvolBox;
break;
case 2:
vesselSolid = vesselBox;
gasvolSolid = gasvolUnion;
Copy link
Contributor

Choose a reason for hiding this comment

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

Add default case (even though already handled, but code above may change and leave this as the first switch).

src/DRICH_geo.cpp Outdated Show resolved Hide resolved
Comment on lines +590 to +593
} // end if(debugOpticsMode != 4)

// large sensor placement (for focal point testing):
if (debugOpticsMode == 4){
Copy link
Contributor

Choose a reason for hiding this comment

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

else ?

@wdconinc
Copy link
Contributor

@cpecar @c-dilks What is the status on this PR? Do we want to complete this before end of March to include in campaign?

@c-dilks
Copy link
Member

c-dilks commented Mar 22, 2023

@cpecar @c-dilks What is the status on this PR? Do we want to complete this before end of March to include in campaign?

This isn't needed for the campaign; it supports an ongoing study for sensor placement optimization.

@cpecar there are still some review comments to resolve.

@wdconinc
Copy link
Contributor

wdconinc commented Oct 1, 2023

Is this branch still under development? Can you rebase, fix merge conflicts, and comment when it's ready?

@wdconinc
Copy link
Contributor

Is this branch still under development?

Adding @chchatte92. Can you pick this up if needed, or close if not needed?

@cpecar
Copy link
Author

cpecar commented Oct 14, 2023

Sorry, I should have finished this a while ago and the PR has fallen behind a a good bit in terms of how the sensors are now defined. I’ll wait for word from Chandra on if this would still be a useful feature and if so I’ll bring it up to date and finalize it.

@rahmans1
Copy link
Contributor

rahmans1 commented Feb 7, 2024

@cpecar @chchatte92 We are tagging a new release for February simulation campaign? What's the status of this PR? Is it supposed to be merged or no?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: forward Positive-rapidity detectors (hadron-going side) topic: PID Particle identification
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

4 participants