WCSim PMT tilt implemented on a per-PMT basis - #25
Conversation
| G4ThreeVector PMTPosition_tilt(pmt_x_shift,pmt_y_shift,tilt_pmt_z_shift); | ||
|
|
||
| // only rotate the affected PMTs (ignore Top, Bottom, and ANNIE Hamamatsu PMTs) | ||
| if ((pmt_type == 3)||(pmt_type == 0 && panel_nr != 0)){ |
There was a problem hiding this comment.
I kept the logic of only applying the angles to the affected PMT populations. This can probably be dropped however there's no reason to tilt the Top/Bottom/Hamamatsu PMTs.
| pmt_y_shift = (168.1-pmt_z)*cm; | ||
| pmt_z_shift = ((pmt_y+14.45))*cm; | ||
| //pmt_z_shift = ((pmt_y+14.45)-InnerStructureCentreOffset/10.)*cm; | ||
| tilt_pmt_z_shift = (pmt_y+14.45+13.9)*cm; |
There was a problem hiding this comment.
why was this 13cm offset added in?
is it angle-dependent?
| double verticalSpacingLAPPD = mainAnnulusHeight/(WCLAPPDperCellVertical+1); | ||
|
|
||
| //G4cout<<"verticalSpacingLAPPD was: " << verticalSpacingLAPPD <<G4endl; | ||
| verticalSpacingLAPPD = 550; |
There was a problem hiding this comment.
can we get a comment on why the hard-coded change?
| //static G4float wavelength_value[20] = { 280., 300., 320., 340., 360., 380., 400., 420., 440., 460., 480., 500., 520., 540., 560., 580., 600., 620., 640., 660.}; | ||
|
|
||
| // wavelength bin for LAPPD 25 for 40 | ||
| static G4float wavelength_value[20] = {220., 240., 260., 280., 300., 320., 340., 360., 380., 400., 420., 440., 460., 480., 500., 520., 540., 560., 580., 600.}; |
There was a problem hiding this comment.
it probably ought to be mentioned in the git commit logs if this is altering PMT quantum efficiency responses as well....
|
|
||
| G4float* LAPPD::GetQE(){ | ||
| // new data from LAPPD 25 for 40 | ||
| static G4float QE[20] = {0.02937435, 0.06062565, 0.09187696 , 0.12312826 , 0.15437956 , 0.18471379 , 0.21641814 , 0.2428098 , 0.24469183 , 0.22234098 , 0.207122 ,0.19884722 , 0.17932031 , 0.14470496 ,0.1113269 , 0.09028386 , 0.07672619 ,0.06275337 , 0.05160584 , 0.03901694}; |
| G4float LAPPD::GetmaxQE(){ | ||
| const G4float maxQE = 0.15; //for LAPPDs //0.211; if for PMTs | ||
|
|
||
| const G4float maxQE = 0.25; //for LAPPDs //0.211; if for PMTs |
There was a problem hiding this comment.
this is also likely to impact LAPPD response potentially significantly (althought it looks like the previous value was too low... i'm not sure why it's even a constant, it probably ought to be calculated).
| // Barrel PMTs have panel numbers 1-8 | ||
| for(int facei=0; facei<WCBarrelRingNPhi; facei++){ | ||
| G4RotationMatrix* WCPMTRotationNext = new G4RotationMatrix(*WCPMTRotation); | ||
| G4RotationMatrix* WCPMTtiltRotationNext = new G4RotationMatrix(*WCPMTRotation); |
There was a problem hiding this comment.
this has the same memory leaks james' version did...
|
|
||
|
|
||
| //G4cout<< -mainAnnulusHeight/2. << " " << (j-1.)*verticalSpacingLAPPD<< G4endl; | ||
| G4ThreeVector LAPPDPosition = G4ThreeVector(CellCentreX, CellCentreY, -119.2+(j-1.)*verticalSpacingLAPPD); |
There was a problem hiding this comment.
this has also had a change to hard-coded value....
WCSim was originally designed to be generative geometry - it has configuration options for variables such as detector height, diameter, cell size etc, and it builds a geometry from that.
We subverted it somewhat by adding an alternative means to place PMTs from a list of positions in file.
But hard-coded values are neither, and should not really be in here....
| G4RotationMatrix* WCPMTtiltRotationNext = new G4RotationMatrix(*WCPMTRotation); | ||
| WCPMTRotationNext->rotateX((dPhi*facei)-67.5*deg+180*deg); | ||
| WCPMTtiltRotationNext->rotateX((dPhi*facei)-67.5*deg+180*deg); | ||
| WCPMTtiltRotationNext->rotateY(-53*deg); |
There was a problem hiding this comment.
This seems to hard-code a rotation angle of -53 degrees, which is then applied on top of the rotation angle added in from file? Why? Doesn't this mean specifying an angle of 0 will result in a tilt?
|
(1) i agree in principle there's no reason to only apply the tilt for barrel PMTs in a per-PMT tilt specification, since one can just set those tilts to 0. Ideally we would remove it, which should be harmless if the implementation is done right. |
2 participants
Very similar to PR #24 that was closed, and includes PMT tilting + LAPPD changes by Yue. Rather than applying a global tilt angle to the WB + WM PMTs, I have added an additional column in
PMTPositions_Scan.txtwhere the user can edit the individual PMT tilt angles based on GoPro observations / other data analyses.Right now all affected PMT tilt values are set to 53 degrees, which is consistent with what James and I have used in our CC and NC cross section analyses.
There are two limitations that should be added but unfortunately I don't have a ton of time to address them - I will try my best to fix them either in this PR or in another in the future: