Use kDefaultRWType for the default ReweightType_t

[jedori0228]

kDefault is too generic and might be mis-used by other enums.

[miquelnebot]

trigger build LArSoft/lar*@LARSOFT_SUITE_v09_82_02

[miquelnebot]

@jedori0228 reviewer?

[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 Failed at phase unit_test SBND on slf7 for c14:prof -- details available through the CI dashboard

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

parent CI build details are 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 ICARUS Warning at phase ci_tests ICARUS on slf7 for e26:prof -- details available through the CI dashboard

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

parent CI build details are available through the CI dashboard

[FNALbuild]

❌ CI build for ICARUS Failed at phase ci_tests ICARUS 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 ICARUS phase logs

parent CI build details are available through the CI dashboard

[PetrilloAtWork]

I rely on the C.I. build system to confirm that there is no breakage here.
I point out that an alternative is to use a scoped enumerator (enum class instead of just enum). This will force prepending the type when using the enumerators (caf::ReweightType_t::kDefault instead of caf::kDefault), and starting from C++20 it can be omitted in specific scopes by a using directive (using enum caf::ReweightType_t;).

[jedori0228]

I rely on the C.I. build system to confirm that there is no breakage here.
I point out that an alternative is to use a scoped enumerator (enum class instead of just enum). This will force prepending the type when using the enumerators (caf::ReweightType_t::kDefault instead of caf::kDefault), and starting from C++20 it can be omitted in specific scopes by a using directive (using enum caf::ReweightType_t;).

@PetrilloAtWork Is it okay to merge this PR for now, and creating an issue with it?

[miquelnebot]

trigger build

[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 Failed at phase unit_test SBND on slf7 for c14:prof -- details available through the CI dashboard

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

parent CI build details are available through the CI dashboard

[FNALbuild]

❌ CI build for SBND Failed at phase unit_test SBND on slf7 for e26:prof -- details available through the CI dashboard

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

parent CI build details are available through the CI dashboard

[FNALbuild]

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

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

parent CI build details are available through the CI dashboard

[FNALbuild]

❌ CI build for ICARUS Failed at phase ci_tests ICARUS 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 ICARUS phase logs

parent CI build details are available through the CI dashboard

[miquelnebot]

@jedori0228 could you check this?
I see SBND e26 CI fails with

59: The following tests FAILED:
60: 10 - fast_simulation_reconstruction_test_electrons.sh (Failed)
61: 11 - fast_simulation_reconstruction_test_muons.sh (Failed)
62: Errors while running CTest
63: Output from these tests are in: /scratch/workspace/sbnd_ci/label_exp/SLF7/label_exp2/swarm/SBND/build_slf7.x86_64/Testing/Temporary/LastTest.log
64: Use "--rerun-failed --output-on-failure" to re-run the failed cases verbosely.
65:
66:
67: FATAL ERROR: stage test FAILED for MRB project SBND SBND_lar_ci with code 8

[jedori0228]

From https://dbweb8.fnal.gov:8443/LarCI/app/ns:sbnd/storage/docs/2024/02/21/LastTest%230oJ7TIn.log,

122284: ---- Configuration BEGIN
122285:   make_tool: 
122286:   ---- FatalRootError BEGIN
122287:     Fatal Root Error: TCling::LoadPCM
122288:     ROOT PCM /scratch/workspace/build-larsoft/BUILDTYPE/prof/QUAL/s120a-e26/label1/swarm/label2/SLF7/build/root/v6_26_06b/Linux64bit+3.10-2.17-e26-p3913-prof/lib/libROOTVecOps_rdict.pcm file does not exist
122289:     ROOT severity: 3000
122290:   ---- FatalRootError END
122291:   Exception caught while processing plugin spec: WCLS
122292: ---- Configuration END

it failed to find root library from cvmfs, and I don't think it's related to this PR?

[kjplows]

trigger build

[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 ICARUS Failed at phase build ICARUS on slf7 for c14:prof -- details available through the CI dashboard

🚨 For more details about the failed phase, check the build ICARUS phase logs

parent CI build details are available through the CI dashboard

[FNALbuild]

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

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

parent CI build details are available through the CI dashboard

[FNALbuild]

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

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

parent CI build details are available through the CI dashboard

[kjplows]

@jedori0228 I notice upon inspecting the code change (through a comment in the source code) there is an associated enum in sbnobj in Common/SBNEventWeight/EventWeightParameterSet.h, class EventWeightParameterSet --> enum EventWeightParameterSet::ReweightType that should be synchronised with this change.
I will make a companion PR to sbnobj (sbnobj/#128) to incorporate this. I am not aware of any other places where the ReweightType_t enum needs to be synchronised, please check if there is any such place. Thanks!

Edit: Yes there is a place like that. See sbncode/SBNEventWeight/Calculators/BNBFlux/FluxCalcPrep.cxx . Companion PR is sbncode/#530

I will halt the merging process until a full review has been done. @PetrilloAtWork apologies for re-requesting your review, but this should ideally be looked at by more than just me.
Specifically, the PR at sbnobj/#128 should be understood and especially why we have duplicate enums in the first place.

[PetrilloAtWork]

It still looks ok to me.

[kjplows]

trigger build LArSoft/larsoft@LARSOFT_SUITE_v10_06_00_02 LArSoft/larwirecell@LARSOFT_SUITE_v10_06_00_02 LArSoft/lar*@LARSOFT_SUITE_v10_06_00

[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 Failed at phase build SBND on slf7 for c14:prof -- details available through the CI dashboard

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

parent CI build details are available through the CI dashboard

[FNALbuild]

❌ CI build for ICARUS Failed at phase build ICARUS on slf7 for c14:prof -- details available through the CI dashboard

🚨 For more details about the failed phase, check the build ICARUS phase logs

parent CI build details are available through the CI dashboard

[FNALbuild]

⚠️ CI build for SBND Warning at phase ci_tests SBND on slf7 for e26:prof - ignored warnings for build -- 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 ICARUS Failed at phase ci_tests ICARUS on slf7 for e26:prof - ignored warnings for build -- details available through the CI dashboard

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

parent CI build details are available through the CI dashboard