Skip to content

Conversation

@brucehoward-physics
Copy link
Contributor

Cherry-picking in what I think are the relevant commits from Gray's feature branch used in the reprocessing.

A few minor edits where necessary to resolve a cherry-pick conflict may be present.

Actually I notice a few things here that relate to GENIE systematics, e.g. config_GENIEReWeightMultisigma_icarus_v2.fcl -- tagging @jedori0228 in case that is already in a commit he has on our board or this or others aren't needed or etc.

@jedori0228
Copy link
Contributor

File changes under fcl/caf/SystTools are covered by #686

…commits) to then be able to merge (hopefully) Jaesung's area
…yst-updates_v09_72_00_06' into feature/howard_forNuMI2023A_from_gputnam-configs
@brucehoward-physics
Copy link
Contributor Author

Merged Jaesung's feature branch into this -- this will facilitate testing and makes it slightly easier/fewer things.

@brucehoward-physics
Copy link
Contributor Author

I believe this should now have the geant4weight stuff removed. I pushed another branch feature/howard_forNuMI2023A_from_gputnam-configs_WITHg4rw which doesn't have these last 2 removal pieces.

@brucehoward-physics
Copy link
Contributor Author

Assigning a reviewer. Sorry Gianluca! 😬 😬

Copy link
Member

@PetrilloAtWork PetrilloAtWork left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have left comments on the parts where my own expertise is not naught, but I can't review the systematics variations.
Somebody else should (@jzettle? @jedori0228?).

In general, there is some thing that DB people should cross check, and I would like references to be added to the numbers in the configurations.

# Recombination fixed, muon+proton fit
CalConstants: [80.32, 79.82, 82.24, 81.68]
# Relative normalization of the TPCs
CalConstants: [1.0118, 1.0000, 1.0333, 1.0230]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Magic numbers! reference?

Comment on lines +78 to +79
icarus_simwire_wirecell_fitSR.wcls_main.structs.gain0: 11.9918701 # mV/fC
icarus_simwire_wirecell_fitSR.wcls_main.structs.gain1: 12.6181926 # mV/fC
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reference?

icarus_data_calconst: [1., 1., 1.]
# Gain with angular dep. recombination
# Assume equal on planes -- this is __wrong__ -- will need to be fixed when they are calibrated
icarus_data_calconst: [0.013316, 0.013316, 0.013316]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reference?

Comment on lines +61 to +62
icarus_calonormtools: [@local::driftnorm, @local::yznorm, @local::tpcgain]
# icarus_calonormtools: [@local::driftnorm_sql, @local::yznorm_sql, @local::tpcgain_sql]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe @jedori0228 or @SFBayLaser can comment on this change? I suppose it is either very right or very wrong, but I never remember which one.

const recob::Hit &hit, const geo::Point_t &location, const geo::Vector_t &direction, double t0) {
// Get the info
ScaleInfo i = GetScaleInfo(e.time().timeHigh());
ScaleInfo i = GetScaleInfo(e.id().runID().run());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Skip the middle men!

Suggested change
ScaleInfo i = GetScaleInfo(e.id().runID().run());
ScaleInfo i = GetScaleInfo(e.run());

Comment on lines +142 to +143
fScaleInfos[run] = thisscale;
return fScaleInfos.at(run);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or:

Suggested change
fScaleInfos[run] = thisscale;
return fScaleInfos.at(run);
return fScaleInfos[run] = thisscale;

(but it may confuse people)

Comment on lines +216 to +217
fScaleInfos[run] = thisscale;
return fScaleInfos.at(run);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above:

Suggested change
fScaleInfos[run] = thisscale;
return fScaleInfos.at(run);
return fScaleInfos[run] = thisscale;

@miquelnebot
Copy link
Contributor

trigger build

@FNALbuild
Copy link
Collaborator

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

@FNALbuild
Copy link
Collaborator

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

@FNALbuild
Copy link
Collaborator

❌ CI build for ICARUS Failed at phase build ICARUS on slf7 for e26: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
Copy link
Collaborator

❌ 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
Copy link
Collaborator

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

@FNALbuild
Copy link
Collaborator

⚠️ CI build for ICARUS Warning at phase ci_tests ICARUS on slf7 for e20:prof - ignored failure for unit_test -- 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

Copy link
Member

@PetrilloAtWork PetrilloAtWork left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. 🤞

@miquelnebot miquelnebot merged commit c4cc7b5 into release/SBN2023A_NuMI May 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment