Skip to content

Conversation

@sungbinoh
Copy link
Contributor

@sungbinoh sungbinoh commented Aug 27, 2025

adding efield and phi to SRCaloPoint for 2025A

This is for the sbncode PR559.

@kjplows
Copy link
Contributor

kjplows commented Aug 27, 2025

trigger build ci_ref=v10_06_02 LArSoft/larsoft@LARSOFT_SUITE_v10_06_00_02 LArSoft/larwirecell@LARSOFT_SUITE_v10_06_00_02 LArSoft/lar*@LARSOFT_SUITE_v10_06_00 SBNSoftware/sbndcode@v10_06_03 SBNSoftware/sbncode@v10_06_00_02

@kjplows kjplows moved this to Testing in SBN software development Aug 27, 2025
@sungbinoh sungbinoh self-assigned this Aug 27, 2025
@sungbinoh sungbinoh added the enhancement New feature or request label Aug 27, 2025
@sungbinoh sungbinoh requested a review from jzennamo August 27, 2025 17:16
@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 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 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

❌ CI build for SBND Failed at phase build SBND on slf7 for e26: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

@kjplows
Copy link
Contributor

kjplows commented Aug 27, 2025

trigger build ci_ref=v10_06_02 LArSoft/larsoft@LARSOFT_SUITE_v10_06_00_01 LArSoft/larwirecell@LARSOFT_SUITE_v10_06_00_01 LArSoft/lar*@LARSOFT_SUITE_v10_06_00 SBNSoftware/sbndcode@v10_06_03

@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 Failed at phase build SBND on slf7 for e26: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 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

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.

Requested a documentation fix.

Comment on lines 23 to 24
float efield; //! |E| [kV/cm]
float phi; //! angle between the E-field and track dir for the hit, used in the GnocchiCalorimetry module
Copy link
Member

Choose a reason for hiding this comment

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

Please use the marker //!< instead of //! (the latter will stick the comment to the next line).

Suggested change
float efield; //! |E| [kV/cm]
float phi; //! angle between the E-field and track dir for the hit, used in the GnocchiCalorimetry module
float efield; //!< |E| [kV/cm]
float phi; //!< angle between the E-field and track dir for the hit, used in the GnocchiCalorimetry module

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi Gianluca, thank you for the review! It is updated!

Comment on lines 23 to 24
float efield; //! |E| [kV/cm]
float phi; //! angle between the E-field and track dir for the hit, used in the GnocchiCalorimetry module
Copy link
Member

Choose a reason for hiding this comment

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

The angle is in radians, right? if not, please document it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi Gianluca, thank you for the review! It is updated!

@PetrilloAtWork
Copy link
Member

Thank you!
I am withholding the approval until the parent pull request (SBNSoftware/sbncode#559) is approved, since it may have consequences on the class in this PR too.

@PetrilloAtWork
Copy link
Member

If SBNSoftware/sbncode#559 is merged as it is today, the unit of the phi data member needs to be updated in the comment.
After that is done, please request review again for final approval of this PR.

@sungbinoh
Copy link
Contributor Author

Hi @PetrilloAtWork , unit for phi in the comment has updated to radian from degree!

@PetrilloAtWork PetrilloAtWork self-requested a review September 4, 2025 00:54
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.

Approved!

@kjplows
Copy link
Contributor

kjplows commented Sep 4, 2025

trigger build ci_ref=v10_06_02 LArSoft/larsoft@LARSOFT_SUITE_v10_06_00_02 LArSoft/larwirecell@LARSOFT_SUITE_v10_06_00_02 LArSoft/lar*@LARSOFT_SUITE_v10_06_00 SBNSoftware/sbn*@release/SBN2025A SBNSoftware/sbndcode@v10_06_03

@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

@kjplows kjplows merged commit e6adc85 into SBNSoftware:release/SBN2025A Sep 5, 2025
3 of 6 checks passed
@github-project-automation github-project-automation bot moved this from Testing to Done in SBN software development Sep 5, 2025
@kjplows kjplows moved this from Done to 2025 PRs in SBN software development Jan 16, 2026
@github-project-automation github-project-automation bot moved this from Todo to Done in PR archaeology Jan 16, 2026
@kjplows kjplows added this to the SBN2025A/v10_06_00_05 milestone Jan 16, 2026
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
Status: 2025 PRs
Status: Todo

Development

Successfully merging this pull request may close these issues.

4 participants