Skip to content

Updates to semi-analytical light simulation: optical path tool#58

Merged
lgarren merged 2 commits intoLArSoft:developfrom
pgreen135:feature/pgreen_optical_path_tool
Aug 15, 2025
Merged

Updates to semi-analytical light simulation: optical path tool#58
lgarren merged 2 commits intoLArSoft:developfrom
pgreen135:feature/pgreen_optical_path_tool

Conversation

@pgreen135
Copy link
Copy Markdown
Contributor

PR to make semi-analytical model constructor call compatible with new optical path tool.

Corresponds with: LArSoft/larsim#157

@FNALbuild
Copy link
Copy Markdown
Contributor

A new Pull Request was created by @pgreen135 (Patrick Green) for develop.

It involves the following packages:

larwirecell

@LArSoft/level-1-managers, @LArSoft/level-2-managers can you please review it and eventually sign? Thanks.

cms-bot commands are listed here

@FNALbuild
Copy link
Copy Markdown
Contributor

The code-checks are being triggered in jenkins.

@FNALbuild
Copy link
Copy Markdown
Contributor

-code-checks
Pull request failed code-formatting checks. Please ensure that cetmodules has been setup and execute the following command from the top-level directory of your repository:

format-code \
  larwirecell/qlmatch/QLMatching.cxx

Then commit the changes and push them to your PR branch.

@FNALbuild
Copy link
Copy Markdown
Contributor

Pull request #58 was updated. @LArSoft/level-1-managers, @LArSoft/level-2-managers can you please check and sign again.

@FNALbuild
Copy link
Copy Markdown
Contributor

The code-checks are being triggered in jenkins.

@FNALbuild
Copy link
Copy Markdown
Contributor

+code-checks

@lynnt20
Copy link
Copy Markdown
Contributor

lynnt20 commented Aug 8, 2025

Changes look good to me!

@FNALbuild
Copy link
Copy Markdown
Contributor

The tests are being triggered in jenkins.
Tested with other pull request(s) LArSoft/larsim#157,SBNSoftware/sbndcode#756,DUNE/dunesw#186,DUNE/duneopdet#104

@lgarren
Copy link
Copy Markdown
Member

lgarren commented Aug 8, 2025

@brettviren @HaiwangYu This PR is needed to match changes in LArSoft/larsim#157. Please review and comment or approve.

@FNALbuild
Copy link
Copy Markdown
Contributor

@FNALbuild
Copy link
Copy Markdown
Contributor

@FNALbuild
Copy link
Copy Markdown
Contributor

@FNALbuild
Copy link
Copy Markdown
Contributor

@FNALbuild
Copy link
Copy Markdown
Contributor

@FNALbuild
Copy link
Copy Markdown
Contributor

-ICARUS tests failed, with build warning,, with ignored warning for build, on slf7 for e26:prof
for details see
https://lar-ci-history.fnal.gov/LarCI/app/ns:ICARUS/view_builds/index?offset=0&builds=icarus_ci/12660&builds=
for details of the parent CI build see
https://lar-ci-history.fnal.gov/LarCI/app/view_builds/index?offset=0&builds=lar_ci/27052&builds=

@brettviren
Copy link
Copy Markdown
Member

It looks fine to me code-wise but I'm not following this development closely. @HaiwangYu's input is worth more than mine here.

@FNALbuild
Copy link
Copy Markdown
Contributor

The tests are being triggered in jenkins.
Tested with other pull request(s) LArSoft/larsim#157,SBNSoftware/sbndcode#756,DUNE/dunesw#193,DUNE/duneopdet#110

@FNALbuild
Copy link
Copy Markdown
Contributor

@FNALbuild
Copy link
Copy Markdown
Contributor

@FNALbuild
Copy link
Copy Markdown
Contributor

@FNALbuild
Copy link
Copy Markdown
Contributor

@FNALbuild
Copy link
Copy Markdown
Contributor

-ICARUS tests failed, with build warning,, with ignored warning for build, on slf7 for e26:prof
for details see
https://lar-ci-history.fnal.gov/LarCI/app/ns:ICARUS/view_builds/index?offset=0&builds=icarus_ci/12692&builds=
for details of the parent CI build see
https://lar-ci-history.fnal.gov/LarCI/app/view_builds/index?offset=0&builds=lar_ci/27089&builds=

@FNALbuild
Copy link
Copy Markdown
Contributor

@lgarren
Copy link
Copy Markdown
Member

lgarren commented Aug 14, 2025

approve

@FNALbuild
Copy link
Copy Markdown
Contributor

This pull request is fully signed and it will be merged to develop and built in the next LArSoft release after it passes the integration tests.

@lgarren lgarren moved this from Approval in progress to Approved and pending inclusion in release in LArSoft pull requests Aug 14, 2025
@lgarren lgarren merged commit 1f2e07d into LArSoft:develop Aug 15, 2025
1 check passed
@github-project-automation github-project-automation Bot moved this from Approved and pending inclusion in release to Merged into develop in LArSoft pull requests Aug 15, 2025
@lgarren lgarren moved this from Merged into develop to Included in release in LArSoft pull requests Aug 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Included in release

Development

Successfully merging this pull request may close these issues.

5 participants