Electron lifetime to 100 ms in wcsimsp_sbnd.fcl & detectorproperties_sbnd.fcl

[annab101]

Description

Changed the electron lifetime from 10.0 ms to 100.0 ms in wcsimsp_sbnd.fcl & detectorproperties_sbnd.fcl

Checklist

• [V] Added at least 1 label from available labels.
• [V] Assigned at least 1 reviewer under Reviewers,
• [V] Assigned all contributers including yourself under Assignees
• Linked any relevant issues under Developement
• Does this PR affect CAF data format? If so, please assign a CAF maintainer as additional reviewer. (I don't think so)
• Does this affect the standard workflow? (I don't think so)

Relevant PR links (optional)

Does this PR require merging another PR in a different repository (such as sbnanobj/sbnobj etc.)? No

Link(s) to docdb describing changes (optional)

Is there a docdb describing the issue this solves or the feature added? docdb 37556-v1 slides 15 & 22 show that lifetime from data is high so simulating a lifetime higher than 10 ms will be important when processing data (https://sbn-docdb.fnal.gov/cgi-bin/sso/ShowDocument?docid=37556)

[weihythu]

Looks good to me.
However, the two changes look inconsistent.
In detector property tcl, the unit is microsecond (us) by default. In wirecell fcl, the unit is millisecond (ms) by default.

Sorry, I noticed the value times 1e3 in property fcl. Ignore my comment then.

[marcodeltutto]

Looks good, thanks!

[bear-is-asleep]

trigger build LArSoft/lar*@LARSOFT_SUITE_v09_91_02

[FNALbuild]

✔️ CI build for LArSoft Succeeded on slf7 for c14:prof -- details available through the CI dashboard

[FNALbuild]

✔️ CI build for LArSoft Succeeded on slf7 for e26:prof -- details available through the CI dashboard

[FNALbuild]

⚠️ CI build for SBND Warning at phase ci_tests SBND on slf7 for e26:prof -- details available through the CI dashboard

🚨 For more details about the warning phase, check the ci_tests SBND phase logs

parent CI build details are available through the CI dashboard

[FNALbuild]

❌ CI build for SBND Failed at phase ci_tests SBND on slf7 for c14:prof - ignored failure for unit_test -- details available through the CI dashboard

🚨 For more details about the failed phase, check the ci_tests SBND phase logs

parent CI build details are available through the CI dashboard

[absolution1]

Hi @annab101
Thanks for making the change.

I think now might be a good time to address there being multiple points of maintenance.

@weihythu is it possible to (easily) change the units used for the electron lifetime (I believe we did this in DUNE-FD)?
If we can use the same units in wirecell/larsoft for the electron lifetime, then we can pin wirecell's value to larsoft

-----#in wcsimpsp_sbnd.fcl--------
#include "detectorproperties_sbnd.fcl"
....
lifetime: @local::sbnd_detproperties.Electronlifetime
....

We have done this in DUNE FD's workflow: https://github.com/DUNE/dunereco/blob/develop/dunereco/DUNEWireCell/wirecell_dune.fcl#L578

[weihythu]

Hi @annab101 Thanks for making the change.

I think now might be a good time to address there being multiple points of maintenance.

@weihythu is it possible to (easily) change the units used for the electron lifetime (I believe we did this in DUNE-FD)? If we can use the same units in wirecell/larsoft, then we can pin wirecell's value to larsoft

-----#in wcsimpsp_sbnd.fcl--------
#include "detectorproperties_sbnd.fcl"
....
lifetime: @local::sbnd_detproperties.Electronlifetime
....

We have done this in DUNE FD's workflow: https://github.com/DUNE/dunereco/blob/develop/dunereco/DUNEWireCell/wirecell_dune.fcl#L578

@annab101 @absolution1 Yes, we can change the default unit in WC fcl to us. Search std.extVar('lifetime') in directory WireCell/cfg/pgrapher/experiment/sbnd, then you will find the unit * wc.ms after each std.extVar('lifetime'), just replace it with * wc.us. Please update the WC fcl as well to indicate the default unit change (from millisecond to microsecond).

[linyan-w]

That's very important, but I'd propose to postpone the organization work to another PR. Anna's on vacation recently, and we want the default e-lifetime changed for the coming production.

[bear-is-asleep]

That's very important, but I'd propose to postpone the organization work to another PR. Anna's on vacation recently, and we want the default e-lifetime changed for the coming production.

Hi @linyan-w , this will probably not be merged for the production being used for the workshop next week as it's a bit late. I will keep this in mind to have merged for a potential patch release if we decide to do another large production, which will likely be happening soon.

[linyan-w]

Yes, I meant for the coming large production, not for the workshop.

[bear-is-asleep]

That's very important, but I'd propose to postpone the organization work to another PR. Anna's on vacation recently, and we want the default e-lifetime changed for the coming production.

Hi all, I agree with Linyan with this. For now this is needed for an imminent production and the production group will need a patch release with this change soon to start testing. I recommend we start another PR to refactor the detsim workflow, similar to what Dom has done with the reco stages. I appreciate that we would like to establish better standards and would be happy to help facilitate this process where I can.

[linyan-w]

Indeed. Also note that there're more places where electron lifetime is defined and assigned value but not really used because WC does not take the parameter value there. This is an example.

[bear-is-asleep]

Indeed. Also note that there're more places where electron lifetime is defined and assigned value but not really used because WC does not take the parameter value there. This is an example.

This should be fine, we can fix these in the g4 refactor PR.

[bear-is-asleep]

Merging with the knowledge that fhicls will need to be refactored according to issue #515 .