search

JeffersonLab / sim-recon

Simulation and Reconstruction for GlueX
9 stars 14 forks source link

Made timing cuts accessible via gPARMS #1114

Closed nsjarvis closed 6 years ago

nsjarvis commented 6 years ago

Added gPARMS so that the LowTCut and HighTCut which Beni added earlier can be accessed through the command line to override the values in CCDB.

JANA flags these as possible typos. CDCHit:DisableTimingCuts has the same issue.

gluex commented 6 years ago

Test status for this pull request: SUCCESS

Summary: /work/halld/pull_request_test/sim-recon^nsj_cdc_timing_cut_gparms/tests/summary.txt Logs: /work/halld/pull_request_test/sim-recon^nsj_cdc_timing_cut_gparms/tests/log

Build log: /work/halld/pull_request_test/sim-recon^nsj_cdc_timing_cut_gparms/make_nsj_cdc_timing_cut_gparms.log Build report: /work/halld/pull_request_test/sim-recon^nsj_cdc_timing_cut_gparms/report_nsj_cdc_timing_cut_gparms.txt Location of build: /work/halld/pull_request_test/sim-recon^nsj_cdc_timing_cut_gparms

sdobbs commented 6 years ago

The help text for CDCHit:HighTCut is not quite right =)

Also, the logic for setting them doesn't quite make sense to me - you are required to set both the high and low cuts if you use them. I would have them default to some ridiculously high or low value and only load the default values if both of them were unchanged (being careful about float point comparisons).

zihlmann commented 6 years ago

I would suggest to just move the gPARM calls after the code where the data base is read then that is all that is needed

cheers, Beni

On 04/25/2018 05:10 PM, Sean Dobbs wrote:

The help text for CDCHit:HighTCut is not quite right =)

Also, the logic for setting them doesn't quite make sense to me - you are required to set both the high and low cuts if you use them. I would have them default to some ridiculously high or low value and only load the default values if both of them were unchanged (being careful about float point comparisons).

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_JeffersonLab_sim-2Drecon_pull_1114-23issuecomment-2D384435146&d=DwMFaQ&c=lz9TcOasaINaaC3U7FbMev2lsutwpI4--09aP8Lu18s&r=o4lft6oWsUt3fPjSD1CFMTzi5N-H01oqTJ5vLjRalTE&m=zUAV3FZkwFpEwHeR5BOeO5G_gzUrsIWmZLWzjd5gTS8&s=PG-2RCMlPMQ4XtgZQePdfK-A7omR7vXXBxXP-BLX9ds&e=, or mute the thread https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_notifications_unsubscribe-2Dauth_AMvwC2L5lkoiuayVGUfeNWMyFdQhEu-5FKks5tsOY0gaJpZM4TkG9B&d=DwMFaQ&c=lz9TcOasaINaaC3U7FbMev2lsutwpI4--09aP8Lu18s&r=o4lft6oWsUt3fPjSD1CFMTzi5N-H01oqTJ5vLjRalTE&m=zUAV3FZkwFpEwHeR5BOeO5G_gzUrsIWmZLWzjd5gTS8&s=pWeIfgo90BHGbIt3D5nzPwl_N0jdpFE22LJnIkAsdyc&e=.

nsjarvis commented 6 years ago

Your comments have been addressed in the new commit. Thank you!

gluex commented 6 years ago

Test status for this pull request: SUCCESS

Summary: /work/halld/pull_request_test/sim-recon^nsj_cdc_timing_cut_gparms/tests/summary.txt Logs: /work/halld/pull_request_test/sim-recon^nsj_cdc_timing_cut_gparms/tests/log

Build log: /work/halld/pull_request_test/sim-recon^nsj_cdc_timing_cut_gparms/make_nsj_cdc_timing_cut_gparms.log Build report: /work/halld/pull_request_test/sim-recon^nsj_cdc_timing_cut_gparms/report_nsj_cdc_timing_cut_gparms.txt Location of build: /work/halld/pull_request_test/sim-recon^nsj_cdc_timing_cut_gparms

zihlmann commented 6 years ago

Hi, looking at the new code I still do not understand why it has to be so complicated: If you put the two lines gPARMS->SetDefaultParameter("CDCHit:LowTCut", LowTCut, "Minimum acceptable CDC hit time (ns)"); gPARMS->SetDefaultParameter("CDCHit:HighTCut", HighTCut, "Maximum acceptable CDC hit time (ns)");

right AFTER "if statement"  where reading from the data base is done (after line 80?) just before the "Disable_CDC_TimingCuts" then you are done. You do not need anything else not new variables and no additional if statements.

cheers, Beni

On 04/25/2018 04:32 PM, nsjarvis wrote:

Added gPARMS so that the LowTCut and HighTCut which Beni added earlier can be accessed through the command line to override the values in CCDB.

JANA flags these as possible typos. CDCHit:DisableTimingCuts has the same issue.

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

https://github.com/JeffersonLab/sim-recon/pull/1114 https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_JeffersonLab_sim-2Drecon_pull_1114&d=DwMCaQ&c=lz9TcOasaINaaC3U7FbMev2lsutwpI4--09aP8Lu18s&r=o4lft6oWsUt3fPjSD1CFMTzi5N-H01oqTJ5vLjRalTE&m=M7bA68HMw9eR3q5imwYiDhetkbGTcna1LRIqTaImMRg&s=DB1jfQJtgENV1O7cWYK71-VTN1-OZ28X-8qqnYEI4KI&e=

    Commit Summary

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_JeffersonLab_sim-2Drecon_pull_1114&d=DwMCaQ&c=lz9TcOasaINaaC3U7FbMev2lsutwpI4--09aP8Lu18s&r=o4lft6oWsUt3fPjSD1CFMTzi5N-H01oqTJ5vLjRalTE&m=M7bA68HMw9eR3q5imwYiDhetkbGTcna1LRIqTaImMRg&s=DB1jfQJtgENV1O7cWYK71-VTN1-OZ28X-8qqnYEI4KI&e=, or mute the thread https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_notifications_unsubscribe-2Dauth_AMvwC-2DfDT9ImLy193K-5FBFEJWlN1xQKLmks5tsN1dgaJpZM4TkG9B&d=DwMCaQ&c=lz9TcOasaINaaC3U7FbMev2lsutwpI4--09aP8Lu18s&r=o4lft6oWsUt3fPjSD1CFMTzi5N-H01oqTJ5vLjRalTE&m=M7bA68HMw9eR3q5imwYiDhetkbGTcna1LRIqTaImMRg&s=nIFnhipsCf1Vr0_WrTvJtomvHdhZ0UC0FWb7viNufqM&e=.

nsjarvis commented 6 years ago

OK, then how about you write it, please?

We should be able to set the time cuts through gparms. This makes it easy to study time window effects without repeatedly recompiling or messing with ccdb.

The gparms values must override the other cut values. It needs to print out a message confirming that the cut values are in place, because jana flags them as a typo. I don't care if these are more or less important than disabletimingcuts=1 because using both of these is stupid but both need the printout message.

Next priority is the values read in from ccdb, if ccdb is available.

Last priority is the default values hardcoded in the file (isn't this a policy violation?).

Thank you.

Naomi.

On Fri, Apr 27, 2018 at 8:10 AM, zihlmann notifications@github.com wrote:

Hi, looking at the new code I still do not understand why it has to be so complicated: If you put the two lines gPARMS->SetDefaultParameter("CDCHit:LowTCut", LowTCut, "Minimum acceptable CDC hit time (ns)"); gPARMS->SetDefaultParameter("CDCHit:HighTCut", HighTCut, "Maximum acceptable CDC hit time (ns)");

right AFTER "if statement" where reading from the data base is done (after line 80?) just before the "Disable_CDC_TimingCuts" then you are done. You do not need anything else not new variables and no additional if statements.

cheers, Beni

On 04/25/2018 04:32 PM, nsjarvis wrote:

Added gPARMS so that the LowTCut and HighTCut which Beni added earlier can be accessed through the command line to override the values in CCDB.

JANA flags these as possible typos. CDCHit:DisableTimingCuts has the same issue.

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

https://github.com/JeffersonLab/sim-recon/pull/1114 https://urldefense.proofpoint.com/v2/url?u=https- 3A__github.com_JeffersonLab_sim-2Drecon_pull_1114&d=DwMCaQ&c= lz9TcOasaINaaC3U7FbMev2lsutwpI4--09aP8Lu18s&r=o4lft6oWsUt3fPjSD1CFMTzi5N- H01oqTJ5vLjRalTE&m=M7bA68HMw9eR3q5imwYiDhetkbGTcna1LRIqTaImMRg&s= DB1jfQJtgENV1O7cWYK71-VTN1-OZ28X-8qqnYEI4KI&e=

Commit Summary

  • Made timing cuts accessible via gPARMS

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://urldefense.proofpoint.com/v2/url?u=https- 3A__github.com_JeffersonLab_sim-2Drecon_pull_1114&d=DwMCaQ&c= lz9TcOasaINaaC3U7FbMev2lsutwpI4--09aP8Lu18s&r=o4lft6oWsUt3fPjSD1CFMTzi5N- H01oqTJ5vLjRalTE&m=M7bA68HMw9eR3q5imwYiDhetkbGTcna1LRIqTaImMRg&s= DB1jfQJtgENV1O7cWYK71-VTN1-OZ28X-8qqnYEI4KI&e=, or mute the thread https://urldefense.proofpoint.com/v2/url?u=https- 3A__github.com_notifications_unsubscribe-2Dauth_AMvwC-2DfDT9ImLy193K- 5FBFEJWlN1xQKLmks5tsN1dgaJpZM4TkG9B&d=DwMCaQ&c= lz9TcOasaINaaC3U7FbMev2lsutwpI4--09aP8Lu18s&r=o4lft6oWsUt3fPjSD1CFMTzi5N- H01oqTJ5vLjRalTE&m=M7bA68HMw9eR3q5imwYiDhetkbGTcna1LRIqTaImMRg&s= nIFnhipsCf1Vr0_WrTvJtomvHdhZ0UC0FWb7viNufqM&e=.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/JeffersonLab/sim-recon/pull/1114#issuecomment-384951704, or mute the thread https://github.com/notifications/unsubscribe-auth/AO7hVrRfbyq88Xxbwuhpu8DI8MUVputsks5tswq9gaJpZM4TkG9B .

TTT

sdobbs commented 6 years ago

Beni,

I think we usually use gPARMS to set these things in the init() function, not the brun(), which can be called multiple times during program execution. Maybe David can weigh in if this is a JANA-approved way to do things.

---Sean

On Fri, Apr 27, 2018 at 8:10 AM zihlmann notifications@github.com wrote:

Hi, looking at the new code I still do not understand why it has to be so complicated: If you put the two lines gPARMS->SetDefaultParameter("CDCHit:LowTCut", LowTCut, "Minimum acceptable CDC hit time (ns)"); gPARMS->SetDefaultParameter("CDCHit:HighTCut", HighTCut, "Maximum acceptable CDC hit time (ns)");

right AFTER "if statement" where reading from the data base is done (after line 80?) just before the "Disable_CDC_TimingCuts" then you are done. You do not need anything else not new variables and no additional if statements.

cheers, Beni

On 04/25/2018 04:32 PM, nsjarvis wrote:

Added gPARMS so that the LowTCut and HighTCut which Beni added earlier can be accessed through the command line to override the values in CCDB.

JANA flags these as possible typos. CDCHit:DisableTimingCuts has the same issue.

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

https://github.com/JeffersonLab/sim-recon/pull/1114 < https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_JeffersonLab_sim-2Drecon_pull_1114&d=DwMCaQ&c=lz9TcOasaINaaC3U7FbMev2lsutwpI4--09aP8Lu18s&r=o4lft6oWsUt3fPjSD1CFMTzi5N-H01oqTJ5vLjRalTE&m=M7bA68HMw9eR3q5imwYiDhetkbGTcna1LRIqTaImMRg&s=DB1jfQJtgENV1O7cWYK71-VTN1-OZ28X-8qqnYEI4KI&e=

Commit Summary

  • Made timing cuts accessible via gPARMS

File Changes

(33)

Patch Links:

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub < https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_JeffersonLab_sim-2Drecon_pull_1114&d=DwMCaQ&c=lz9TcOasaINaaC3U7FbMev2lsutwpI4--09aP8Lu18s&r=o4lft6oWsUt3fPjSD1CFMTzi5N-H01oqTJ5vLjRalTE&m=M7bA68HMw9eR3q5imwYiDhetkbGTcna1LRIqTaImMRg&s=DB1jfQJtgENV1O7cWYK71-VTN1-OZ28X-8qqnYEI4KI&e=>,

or mute the thread < https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_notifications_unsubscribe-2Dauth_AMvwC-2DfDT9ImLy193K-5FBFEJWlN1xQKLmks5tsN1dgaJpZM4TkG9B&d=DwMCaQ&c=lz9TcOasaINaaC3U7FbMev2lsutwpI4--09aP8Lu18s&r=o4lft6oWsUt3fPjSD1CFMTzi5N-H01oqTJ5vLjRalTE&m=M7bA68HMw9eR3q5imwYiDhetkbGTcna1LRIqTaImMRg&s=nIFnhipsCf1Vr0_WrTvJtomvHdhZ0UC0FWb7viNufqM&e= .

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/JeffersonLab/sim-recon/pull/1114#issuecomment-384951704, or mute the thread https://github.com/notifications/unsubscribe-auth/ABIJahUgBoWxxOM0UWQZVBz5uT1qK1s-ks5tswq8gaJpZM4TkG9B .

zihlmann commented 6 years ago

I just asked Simon and he said gPARMS works also in brun.

On 04/27/2018 09:41 AM, Sean Dobbs wrote:

Beni,

I think we usually use gPARMS to set these things in the init() function, not the brun(), which can be called multiple times during program execution. Maybe David can weigh in if this is a JANA-approved way to do things.

---Sean

On Fri, Apr 27, 2018 at 8:10 AM zihlmann notifications@github.com wrote:

Hi, looking at the new code I still do not understand why it has to be so complicated: If you put the two lines gPARMS->SetDefaultParameter("CDCHit:LowTCut", LowTCut, "Minimum acceptable CDC hit time (ns)"); gPARMS->SetDefaultParameter("CDCHit:HighTCut", HighTCut, "Maximum acceptable CDC hit time (ns)");

right AFTER "if statement" where reading from the data base is done (after line 80?) just before the "Disable_CDC_TimingCuts" then you are done. You do not need anything else not new variables and no additional if statements.

cheers, Beni

On 04/25/2018 04:32 PM, nsjarvis wrote:

Added gPARMS so that the LowTCut and HighTCut which Beni added earlier can be accessed through the command line to override the values in CCDB.

JANA flags these as possible typos. CDCHit:DisableTimingCuts has the same issue.

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

https://github.com/JeffersonLab/sim-recon/pull/1114 <

https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_JeffersonLab_sim-2Drecon_pull_1114&d=DwMCaQ&c=lz9TcOasaINaaC3U7FbMev2lsutwpI4--09aP8Lu18s&r=o4lft6oWsUt3fPjSD1CFMTzi5N-H01oqTJ5vLjRalTE&m=M7bA68HMw9eR3q5imwYiDhetkbGTcna1LRIqTaImMRg&s=DB1jfQJtgENV1O7cWYK71-VTN1-OZ28X-8qqnYEI4KI&e=

Commit Summary

  • Made timing cuts accessible via gPARMS

File Changes

  • M src/libraries/CDC/DCDCHit_factory.cc <

https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_JeffersonLab_sim-2Drecon_pull_1114_files-23diff-2D0&d=DwMCaQ&c=lz9TcOasaINaaC3U7FbMev2lsutwpI4--09aP8Lu18s&r=o4lft6oWsUt3fPjSD1CFMTzi5N-H01oqTJ5vLjRalTE&m=M7bA68HMw9eR3q5imwYiDhetkbGTcna1LRIqTaImMRg&s=8-Kp2B9VUTpjuN3e6NrkIVp30Q6Jom4v4cCpTfSMn0E&e=

(33)

Patch Links:

https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_JeffersonLab_sim-2Drecon_pull_1114.patch&d=DwMCaQ&c=lz9TcOasaINaaC3U7FbMev2lsutwpI4--09aP8Lu18s&r=o4lft6oWsUt3fPjSD1CFMTzi5N-H01oqTJ5vLjRalTE&m=M7bA68HMw9eR3q5imwYiDhetkbGTcna1LRIqTaImMRg&s=92N1KVNGdaTjfDfonAXYx_8_SNmxXhQiT8JhDFwwhGI&e=

https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_JeffersonLab_sim-2Drecon_pull_1114.diff&d=DwMCaQ&c=lz9TcOasaINaaC3U7FbMev2lsutwpI4--09aP8Lu18s&r=o4lft6oWsUt3fPjSD1CFMTzi5N-H01oqTJ5vLjRalTE&m=M7bA68HMw9eR3q5imwYiDhetkbGTcna1LRIqTaImMRg&s=KTfLE8TXR1nLfWr0pwJPgRIRoh4hEyBbUUMMntv3UGc&e=

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <

https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_JeffersonLab_sim-2Drecon_pull_1114&d=DwMCaQ&c=lz9TcOasaINaaC3U7FbMev2lsutwpI4--09aP8Lu18s&r=o4lft6oWsUt3fPjSD1CFMTzi5N-H01oqTJ5vLjRalTE&m=M7bA68HMw9eR3q5imwYiDhetkbGTcna1LRIqTaImMRg&s=DB1jfQJtgENV1O7cWYK71-VTN1-OZ28X-8qqnYEI4KI&e=>,

or mute the thread <

https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_notifications_unsubscribe-2Dauth_AMvwC-2DfDT9ImLy193K-5FBFEJWlN1xQKLmks5tsN1dgaJpZM4TkG9B&d=DwMCaQ&c=lz9TcOasaINaaC3U7FbMev2lsutwpI4--09aP8Lu18s&r=o4lft6oWsUt3fPjSD1CFMTzi5N-H01oqTJ5vLjRalTE&m=M7bA68HMw9eR3q5imwYiDhetkbGTcna1LRIqTaImMRg&s=nIFnhipsCf1Vr0_WrTvJtomvHdhZ0UC0FWb7viNufqM&e=

.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub

https://github.com/JeffersonLab/sim-recon/pull/1114#issuecomment-384951704, or mute the thread

https://github.com/notifications/unsubscribe-auth/ABIJahUgBoWxxOM0UWQZVBz5uT1qK1s-ks5tswq8gaJpZM4TkG9B .

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_JeffersonLab_sim-2Drecon_pull_1114-23issuecomment-2D384973313&d=DwMFaQ&c=lz9TcOasaINaaC3U7FbMev2lsutwpI4--09aP8Lu18s&r=o4lft6oWsUt3fPjSD1CFMTzi5N-H01oqTJ5vLjRalTE&m=euunZ8a4pApQ4egqX6rLtH5S-pBDLyzWYkowzLqGwPs&s=qnyrrlaRUXmJCup3fQrJVuPyx442m7BuLRFmt4ohv80&e=, or mute the thread https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_notifications_unsubscribe-2Dauth_AMvwC6pDia3uOgPw69chD-5FUeY6NWfNLxks5tsyAJgaJpZM4TkG9B&d=DwMFaQ&c=lz9TcOasaINaaC3U7FbMev2lsutwpI4--09aP8Lu18s&r=o4lft6oWsUt3fPjSD1CFMTzi5N-H01oqTJ5vLjRalTE&m=euunZ8a4pApQ4egqX6rLtH5S-pBDLyzWYkowzLqGwPs&s=vnZT4AuKtZ_oXSmbJQaNkj1MW1oaQD-Kba2A2YQw0Ow&e=.

sdobbs commented 6 years ago

OK, never mind, then =)

On Fri, Apr 27, 2018 at 10:18 AM zihlmann notifications@github.com wrote:

I just asked Simon and he said gPARMS works also in brun.

On 04/27/2018 09:41 AM, Sean Dobbs wrote:

Beni,

I think we usually use gPARMS to set these things in the init() function, not the brun(), which can be called multiple times during program execution. Maybe David can weigh in if this is a JANA-approved way to do things.

---Sean

On Fri, Apr 27, 2018 at 8:10 AM zihlmann notifications@github.com wrote:

Hi, looking at the new code I still do not understand why it has to be so complicated: If you put the two lines gPARMS->SetDefaultParameter("CDCHit:LowTCut", LowTCut, "Minimum acceptable CDC hit time (ns)"); gPARMS->SetDefaultParameter("CDCHit:HighTCut", HighTCut, "Maximum acceptable CDC hit time (ns)");

right AFTER "if statement" where reading from the data base is done (after line 80?) just before the "Disable_CDC_TimingCuts" then you are done. You do not need anything else not new variables and no additional if statements.

cheers, Beni

On 04/25/2018 04:32 PM, nsjarvis wrote:

Added gPARMS so that the LowTCut and HighTCut which Beni added earlier can be accessed through the command line to override the values in CCDB.

JANA flags these as possible typos. CDCHit:DisableTimingCuts has the same issue.

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

https://github.com/JeffersonLab/sim-recon/pull/1114 <

https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_JeffersonLab_sim-2Drecon_pull_1114&d=DwMCaQ&c=lz9TcOasaINaaC3U7FbMev2lsutwpI4--09aP8Lu18s&r=o4lft6oWsUt3fPjSD1CFMTzi5N-H01oqTJ5vLjRalTE&m=M7bA68HMw9eR3q5imwYiDhetkbGTcna1LRIqTaImMRg&s=DB1jfQJtgENV1O7cWYK71-VTN1-OZ28X-8qqnYEI4KI&e=

Commit Summary

  • Made timing cuts accessible via gPARMS

File Changes

  • M src/libraries/CDC/DCDCHit_factory.cc <

https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_JeffersonLab_sim-2Drecon_pull_1114_files-23diff-2D0&d=DwMCaQ&c=lz9TcOasaINaaC3U7FbMev2lsutwpI4--09aP8Lu18s&r=o4lft6oWsUt3fPjSD1CFMTzi5N-H01oqTJ5vLjRalTE&m=M7bA68HMw9eR3q5imwYiDhetkbGTcna1LRIqTaImMRg&s=8-Kp2B9VUTpjuN3e6NrkIVp30Q6Jom4v4cCpTfSMn0E&e=

(33)

Patch Links:

https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_JeffersonLab_sim-2Drecon_pull_1114.patch&d=DwMCaQ&c=lz9TcOasaINaaC3U7FbMev2lsutwpI4--09aP8Lu18s&r=o4lft6oWsUt3fPjSD1CFMTzi5N-H01oqTJ5vLjRalTE&m=M7bA68HMw9eR3q5imwYiDhetkbGTcna1LRIqTaImMRg&s=92N1KVNGdaTjfDfonAXYx_8_SNmxXhQiT8JhDFwwhGI&e=

https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_JeffersonLab_sim-2Drecon_pull_1114.diff&d=DwMCaQ&c=lz9TcOasaINaaC3U7FbMev2lsutwpI4--09aP8Lu18s&r=o4lft6oWsUt3fPjSD1CFMTzi5N-H01oqTJ5vLjRalTE&m=M7bA68HMw9eR3q5imwYiDhetkbGTcna1LRIqTaImMRg&s=KTfLE8TXR1nLfWr0pwJPgRIRoh4hEyBbUUMMntv3UGc&e=

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <

https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_JeffersonLab_sim-2Drecon_pull_1114&d=DwMCaQ&c=lz9TcOasaINaaC3U7FbMev2lsutwpI4--09aP8Lu18s&r=o4lft6oWsUt3fPjSD1CFMTzi5N-H01oqTJ5vLjRalTE&m=M7bA68HMw9eR3q5imwYiDhetkbGTcna1LRIqTaImMRg&s=DB1jfQJtgENV1O7cWYK71-VTN1-OZ28X-8qqnYEI4KI&e= ,

or mute the thread <

https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_notifications_unsubscribe-2Dauth_AMvwC-2DfDT9ImLy193K-5FBFEJWlN1xQKLmks5tsN1dgaJpZM4TkG9B&d=DwMCaQ&c=lz9TcOasaINaaC3U7FbMev2lsutwpI4--09aP8Lu18s&r=o4lft6oWsUt3fPjSD1CFMTzi5N-H01oqTJ5vLjRalTE&m=M7bA68HMw9eR3q5imwYiDhetkbGTcna1LRIqTaImMRg&s=nIFnhipsCf1Vr0_WrTvJtomvHdhZ0UC0FWb7viNufqM&e=

.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub

< https://github.com/JeffersonLab/sim-recon/pull/1114#issuecomment-384951704 , or mute the thread

< https://github.com/notifications/unsubscribe-auth/ABIJahUgBoWxxOM0UWQZVBz5uT1qK1s-ks5tswq8gaJpZM4TkG9B

.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub < https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_JeffersonLab_sim-2Drecon_pull_1114-23issuecomment-2D384973313&d=DwMFaQ&c=lz9TcOasaINaaC3U7FbMev2lsutwpI4--09aP8Lu18s&r=o4lft6oWsUt3fPjSD1CFMTzi5N-H01oqTJ5vLjRalTE&m=euunZ8a4pApQ4egqX6rLtH5S-pBDLyzWYkowzLqGwPs&s=qnyrrlaRUXmJCup3fQrJVuPyx442m7BuLRFmt4ohv80&e=>,

or mute the thread < https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_notifications_unsubscribe-2Dauth_AMvwC6pDia3uOgPw69chD-5FUeY6NWfNLxks5tsyAJgaJpZM4TkG9B&d=DwMFaQ&c=lz9TcOasaINaaC3U7FbMev2lsutwpI4--09aP8Lu18s&r=o4lft6oWsUt3fPjSD1CFMTzi5N-H01oqTJ5vLjRalTE&m=euunZ8a4pApQ4egqX6rLtH5S-pBDLyzWYkowzLqGwPs&s=vnZT4AuKtZ_oXSmbJQaNkj1MW1oaQD-Kba2A2YQw0Ow&e= .

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/JeffersonLab/sim-recon/pull/1114#issuecomment-384984130, or mute the thread https://github.com/notifications/unsubscribe-auth/ABIJahepOzfJP1Bq54CPwyEB2c148kfqks5tsyi5gaJpZM4TkG9B .

zihlmann commented 6 years ago

I am testing it right now. If it works I will commit it

On 04/27/2018 10:19 AM, Sean Dobbs wrote:

OK, never mind, then =)

On Fri, Apr 27, 2018 at 10:18 AM zihlmann notifications@github.com wrote:

I just asked Simon and he said gPARMS works also in brun.

On 04/27/2018 09:41 AM, Sean Dobbs wrote:

Beni,

I think we usually use gPARMS to set these things in the init() function, not the brun(), which can be called multiple times during program execution. Maybe David can weigh in if this is a JANA-approved way to do things.

---Sean

On Fri, Apr 27, 2018 at 8:10 AM zihlmann notifications@github.com wrote:

Hi, looking at the new code I still do not understand why it has to be so complicated: If you put the two lines gPARMS->SetDefaultParameter("CDCHit:LowTCut", LowTCut, "Minimum acceptable CDC hit time (ns)"); gPARMS->SetDefaultParameter("CDCHit:HighTCut", HighTCut, "Maximum acceptable CDC hit time (ns)");

right AFTER "if statement" where reading from the data base is done (after line 80?) just before the "Disable_CDC_TimingCuts" then you are done. You do not need anything else not new variables and no additional if statements.

cheers, Beni

On 04/25/2018 04:32 PM, nsjarvis wrote:

Added gPARMS so that the LowTCut and HighTCut which Beni added earlier can be accessed through the command line to override the values in CCDB.

JANA flags these as possible typos. CDCHit:DisableTimingCuts has the same issue.

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

https://github.com/JeffersonLab/sim-recon/pull/1114 <

https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_JeffersonLab_sim-2Drecon_pull_1114&d=DwMCaQ&c=lz9TcOasaINaaC3U7FbMev2lsutwpI4--09aP8Lu18s&r=o4lft6oWsUt3fPjSD1CFMTzi5N-H01oqTJ5vLjRalTE&m=M7bA68HMw9eR3q5imwYiDhetkbGTcna1LRIqTaImMRg&s=DB1jfQJtgENV1O7cWYK71-VTN1-OZ28X-8qqnYEI4KI&e=

Commit Summary

  • Made timing cuts accessible via gPARMS

File Changes

  • M src/libraries/CDC/DCDCHit_factory.cc <

https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_JeffersonLab_sim-2Drecon_pull_1114_files-23diff-2D0&d=DwMCaQ&c=lz9TcOasaINaaC3U7FbMev2lsutwpI4--09aP8Lu18s&r=o4lft6oWsUt3fPjSD1CFMTzi5N-H01oqTJ5vLjRalTE&m=M7bA68HMw9eR3q5imwYiDhetkbGTcna1LRIqTaImMRg&s=8-Kp2B9VUTpjuN3e6NrkIVp30Q6Jom4v4cCpTfSMn0E&e=

(33)

Patch Links:

https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_JeffersonLab_sim-2Drecon_pull_1114.patch&d=DwMCaQ&c=lz9TcOasaINaaC3U7FbMev2lsutwpI4--09aP8Lu18s&r=o4lft6oWsUt3fPjSD1CFMTzi5N-H01oqTJ5vLjRalTE&m=M7bA68HMw9eR3q5imwYiDhetkbGTcna1LRIqTaImMRg&s=92N1KVNGdaTjfDfonAXYx_8_SNmxXhQiT8JhDFwwhGI&e=

https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_JeffersonLab_sim-2Drecon_pull_1114.diff&d=DwMCaQ&c=lz9TcOasaINaaC3U7FbMev2lsutwpI4--09aP8Lu18s&r=o4lft6oWsUt3fPjSD1CFMTzi5N-H01oqTJ5vLjRalTE&m=M7bA68HMw9eR3q5imwYiDhetkbGTcna1LRIqTaImMRg&s=KTfLE8TXR1nLfWr0pwJPgRIRoh4hEyBbUUMMntv3UGc&e=

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <

https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_JeffersonLab_sim-2Drecon_pull_1114&d=DwMCaQ&c=lz9TcOasaINaaC3U7FbMev2lsutwpI4--09aP8Lu18s&r=o4lft6oWsUt3fPjSD1CFMTzi5N-H01oqTJ5vLjRalTE&m=M7bA68HMw9eR3q5imwYiDhetkbGTcna1LRIqTaImMRg&s=DB1jfQJtgENV1O7cWYK71-VTN1-OZ28X-8qqnYEI4KI&e=

,

or mute the thread <

https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_notifications_unsubscribe-2Dauth_AMvwC-2DfDT9ImLy193K-5FBFEJWlN1xQKLmks5tsN1dgaJpZM4TkG9B&d=DwMCaQ&c=lz9TcOasaINaaC3U7FbMev2lsutwpI4--09aP8Lu18s&r=o4lft6oWsUt3fPjSD1CFMTzi5N-H01oqTJ5vLjRalTE&m=M7bA68HMw9eR3q5imwYiDhetkbGTcna1LRIqTaImMRg&s=nIFnhipsCf1Vr0_WrTvJtomvHdhZ0UC0FWb7viNufqM&e=

.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub

<

https://github.com/JeffersonLab/sim-recon/pull/1114#issuecomment-384951704

,

or mute the thread

<

https://github.com/notifications/unsubscribe-auth/ABIJahUgBoWxxOM0UWQZVBz5uT1qK1s-ks5tswq8gaJpZM4TkG9B

.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub <

https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_JeffersonLab_sim-2Drecon_pull_1114-23issuecomment-2D384973313&d=DwMFaQ&c=lz9TcOasaINaaC3U7FbMev2lsutwpI4--09aP8Lu18s&r=o4lft6oWsUt3fPjSD1CFMTzi5N-H01oqTJ5vLjRalTE&m=euunZ8a4pApQ4egqX6rLtH5S-pBDLyzWYkowzLqGwPs&s=qnyrrlaRUXmJCup3fQrJVuPyx442m7BuLRFmt4ohv80&e=>,

or mute the thread <

https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_notifications_unsubscribe-2Dauth_AMvwC6pDia3uOgPw69chD-5FUeY6NWfNLxks5tsyAJgaJpZM4TkG9B&d=DwMFaQ&c=lz9TcOasaINaaC3U7FbMev2lsutwpI4--09aP8Lu18s&r=o4lft6oWsUt3fPjSD1CFMTzi5N-H01oqTJ5vLjRalTE&m=euunZ8a4pApQ4egqX6rLtH5S-pBDLyzWYkowzLqGwPs&s=vnZT4AuKtZ_oXSmbJQaNkj1MW1oaQD-Kba2A2YQw0Ow&e=

.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub

https://github.com/JeffersonLab/sim-recon/pull/1114#issuecomment-384984130, or mute the thread

https://github.com/notifications/unsubscribe-auth/ABIJahepOzfJP1Bq54CPwyEB2c148kfqks5tsyi5gaJpZM4TkG9B .

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_JeffersonLab_sim-2Drecon_pull_1114-23issuecomment-2D384984367&d=DwMFaQ&c=lz9TcOasaINaaC3U7FbMev2lsutwpI4--09aP8Lu18s&r=o4lft6oWsUt3fPjSD1CFMTzi5N-H01oqTJ5vLjRalTE&m=AFVIHEIuKzzMyfIZwHYl0wDrOguxqZhwxcJFz2HKiWU&s=_1eOLcJWw8pCxpGQkCzt_TfdJ2ntdriXyH4zUOFv740&e=, or mute the thread https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_notifications_unsubscribe-2Dauth_AMvwC-5FFPESbKrfd97C45iKpcmfNKOJMPks5tsyjugaJpZM4TkG9B&d=DwMFaQ&c=lz9TcOasaINaaC3U7FbMev2lsutwpI4--09aP8Lu18s&r=o4lft6oWsUt3fPjSD1CFMTzi5N-H01oqTJ5vLjRalTE&m=AFVIHEIuKzzMyfIZwHYl0wDrOguxqZhwxcJFz2HKiWU&s=WfQJlsK3aLJJgLP3ft1kgzUVX0KY5J998jMF1RTLg9M&e=.

sdobbs commented 6 years ago

Merged Beni's suggested changes instead, closing this one...