Skip to content

Conversation

@rtriozzi
Copy link
Contributor

@rtriozzi rtriozzi commented Jul 23, 2025

This PR changes the CAFMaker (particularly, FillPFPVars) to accomodate for changes to the track/shower discrimination scheme. In the updated Pandora track/shower discrimination module, the ConeChargeFeatureTool_ICARUS and ThreeDChargeFeatureTool_ICARUS tools are introduced. The names of the corresponding BDT tools in the CAFMaker need to be updated to pick up the right variables.


Dependency

This PR depends on icaruscode PR #840, which updates the Pandora XML and introduces the two new track/shower discrimination tools. The corresponding icaruscode PR contains all the information on the change.


Review

Tagging for review:

Also tagging @acampani as assignee.

Thanks!

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.

One more problem. 😳

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.

Good to go from my side.

@kjplows kjplows moved this from Open pull requests to Partially reviewed in SBN software development Aug 9, 2025
@kjplows
Copy link
Contributor

kjplows commented Aug 9, 2025

Hi @rtriozzi, could you please merge in develop to your feature branch and check the merge preserves the features you want? Thanks.

@brucehoward-physics, is the PR good to go from your side?

Copy link
Contributor

@brucehoward-physics brucehoward-physics left a comment

Choose a reason for hiding this comment

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

This looks pretty good to me, I think just some of the default names need to be modified. Just a check: has someone verified that this works as expected with SBND?

@brucehoward-physics
Copy link
Contributor

Thanks Riccardo, this looks good now! Only remaining question is if this was tested in both SBND and ICARUS workflows, but maybe CI will be able to test this before merging? If so then I'll approve.

@kjplows
Copy link
Contributor

kjplows commented Aug 26, 2025

trigger build ci_ref=v10_06_02 LArSoft/lar*@LARSOFT_SUITE_v10_09_00 SBNSoftware/icaruscode#840

@FNALbuild
Copy link

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

@FNALbuild
Copy link

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

@FNALbuild
Copy link

❌ 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

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

⚠️ 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
Copy link

❌ 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

@kjplows
Copy link
Contributor

kjplows commented Aug 26, 2025

@brucehoward-physics , from what I can tell (arguably running slightly outdated SBND CI refs) the SBND side is happy with the trigger build. ICARUS CI is broken for the time being so you can ignore failures on ICARUS e26:prof.

@kjplows kjplows moved this from Partially reviewed to Testing in SBN software development Aug 26, 2025
@kjplows
Copy link
Contributor

kjplows commented Aug 26, 2025

Also @cerati would you have any time to look over this if you can? Thanks!

@kjplows kjplows moved this from Testing to Urgent checks in SBN software development Sep 4, 2025
@kjplows kjplows moved this from Urgent checks to Partially reviewed in SBN software development Sep 12, 2025
@kjplows
Copy link
Contributor

kjplows commented Sep 17, 2025

trigger build

@FNALbuild
Copy link

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

@FNALbuild
Copy link

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

@FNALbuild
Copy link

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

❌ 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

❌ 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

@FNALbuild
Copy link

⚠️ 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

Copy link
Contributor

@brucehoward-physics brucehoward-physics left a comment

Choose a reason for hiding this comment

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

Since @kjplows reports the SBND CI passes and @rtriozzi reported testing the ICARUS side, I will hit approve!

@kjplows kjplows moved this from Partially reviewed to To merge in SBN software development Sep 17, 2025
@kjplows kjplows merged commit 7241a86 into develop Sep 18, 2025
@github-project-automation github-project-automation bot moved this from To merge to Done in SBN software development Sep 18, 2025
@kjplows kjplows moved this from Done to Recently done in SBN software development Sep 18, 2025
@kjplows kjplows moved this from Recently done to Done in SBN software development Oct 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

8 participants