search

JeffersonLab / HDGeant4

Geant4 simulation for the GlueX experiment
4 stars 4 forks source link

make FMWPC wire locations compatible with using cm as units #189

Closed zihlmann closed 3 years ago

zihlmann commented 3 years ago

for their location. Note these are still hard coded and will need a proper treatement using ccdb in the near future. This also expects to use 6 chambers. and the last one having vertical wires. Use this in combination with the latest geometry for the chamber which is currently only in hddm not in ccdb. This will be updated soon

rjones30 commented 3 years ago

Beni,

I took the additional step of making these constants symbolic. They are still hard-wired but now they have names and an explanation of their meaning, in anticipation that they might someday be updated from ccdb. Have a look.

-Richard J.

On Tue, Apr 13, 2021 at 8:56 AM zihlmann @.***> wrote:

for their location. Note these are still hard coded and will need a proper treatement using ccdb in the near future. This also expects to use 6 chambers. and the last one having vertical wires. Use this in combination with the latest geometry for the chamber which is currently only in hddm not in ccdb. This will be updated soon

You can view, comment on, or merge this pull request online at:

https://github.com/JeffersonLab/HDGeant4/pull/189 Commit Summary

  • make FMWPC wire locations compatible with using cm as units

File Changes

Patch Links:

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/JeffersonLab/HDGeant4/pull/189, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB3YKWEYHH5U4YF2KNDF7ETTIQ5Q7ANCNFSM423JLEYQ .

zihlmann commented 3 years ago

the code "/cm" got dropped which will result in comparing mm to cm since the x[i] values are in mm not cm.

rjones30 commented 3 years ago

Beni, can you look again? -Richard

On Tue, Apr 13, 2021 at 10:00 AM zihlmann @.***> wrote:

the code "/cm" got dropped which will result in comparing mm to cm since the x[i] values are in mm not cm.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/JeffersonLab/HDGeant4/pull/189#issuecomment-818759473, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB3YKWGI6SP5T3MJDENCZCTTIRE7TANCNFSM423JLEYQ .

zihlmann commented 3 years ago 
   // Vertical wires
     wire = floor(x[0]/cm + 73.0);
     wire = floor(x[0] - WIRE_OFFSET)/WIRE_PITCH;

the difference between these two lines of codes is that in the first we have x[0]/cm to make it to units of "cm" while in the second line we only have x[0] which is in unites of "mm" while the value WIRE_OFFSET is in unites of "cm"

or am I missing something?

eltonssmith commented 3 years ago

Is there a reason that we do not simply do everything in mm? That would make it consistent with the GEANT units. Notes in the code can emphasize that values are in mm.

zihlmann commented 3 years ago

remember many many years ago (when we also still used geant3) we decided to use the units cm, ns, GeV, GeV/c ...... as a result hopefully all values in ccdb are in those units where needed.

rjones30 commented 3 years ago

Beni,

You are missing something here. The definition of WIRE_OFFSET and WIRE_PITCH are

double GlueXSensitiveDetectorFMWPC::WIRE_OFFSET = -73.0cm; double GlueXSensitiveDetectorFMWPC::WIRE_PITCH = 1.0cm;

The "cm" factor has a numerical value of 10. This notation "73cm" means assign this variable a length of 73 cm in whatever length units are in use in this context".

-Richard

On Tue, Apr 13, 2021 at 10:16 AM zihlmann @.***> wrote:

// Vertical wires wire = floor(x[0]/cm + 73.0); wire = floor(x[0] - WIRE_OFFSET)/WIRE_PITCH;

the difference between these two lines of codes is that in the first we have x[0]/cm to make it to units of "cm" while in the second line we only have x[0] which is in unites of "mm" while the value WIRE_OFFSET is in unites of "cm"

or am I missing something?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/JeffersonLab/HDGeant4/pull/189#issuecomment-818772115, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB3YKWAYMQZ5CG7G3ZGVWY3TIRG4TANCNFSM423JLEYQ .

rjones30 commented 3 years ago

The principle in G4 is to be units-agnostic. If you have a constant with units you always write it as value*unit which makes the code much more readable. This does not need to be over-thought -- just follow the G4 convention, it is better than G3 in this regard. -Richard Jones

On Tue, Apr 13, 2021 at 10:20 AM eltonssmith @.***> wrote:

Is there a reason that we do not simply do everything in mm? That would make it consistent with the GEANT units. Notes in the code can emphasize that values are in mm.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/JeffersonLab/HDGeant4/pull/189#issuecomment-818774915, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB3YKWDX7NNVLM5M7WF76RLTIRHKTANCNFSM423JLEYQ .

rjones30 commented 3 years ago

PS. That is, in the G4 code itself. In ccdb, the quantities all need to carry only their units. I suggest a notation like wire_offset_cm or signal_thresh_keV for constants in the ccdb that need to carry units. -Richard

On Tue, Apr 13, 2021 at 10:26 AM Richard Jones @.***> wrote:

The principle in G4 is to be units-agnostic. If you have a constant with units you always write it as value*unit which makes the code much more readable. This does not need to be over-thought -- just follow the G4 convention, it is better than G3 in this regard. -Richard Jones

On Tue, Apr 13, 2021 at 10:20 AM eltonssmith @.***> wrote:

Is there a reason that we do not simply do everything in mm? That would make it consistent with the GEANT units. Notes in the code can emphasize that values are in mm.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/JeffersonLab/HDGeant4/pull/189#issuecomment-818774915, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB3YKWDX7NNVLM5M7WF76RLTIRHKTANCNFSM423JLEYQ .

zihlmann commented 3 years ago

thank you! yes I missed the cm in the definition of the variable.