Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pions #1341

Merged
merged 7 commits into from
Oct 4, 2024
Merged

pions #1341

merged 7 commits into from
Oct 4, 2024

Conversation

sophiemiddleton
Copy link
Contributor

pion filters for RPC

@FNALbuild
Copy link
Collaborator

Hi @sophiemiddleton,
You have proposed changes to files in these packages:

  • EventGenerator

which require these tests: build.

@Mu2e/write, @Mu2e/fnalbuild-users have access to CI actions on main.

⌛ The following tests have been triggered for 7749c0d: build (Build queue has 1 jobs)

About FNALbuild. Code review on Mu2e/Offline.

@sophiemiddleton
Copy link
Contributor Author

Still need to do some work on the filter to tidy it up

@FNALbuild
Copy link
Collaborator

☀️ The build tests passed at 7749c0d.

Test Result Details
test with Command did not list any other PRs to include
merge Merged 7749c0d at 38f72f8
build (prof) Log file. Build time: 04 min 13 sec
ceSimReco Log file.
g4test_03MT Log file.
transportOnly Log file.
POT Log file.
g4study Log file.
cosmicSimReco Log file.
cosmicOffSpill Log file.
ceSteps Log file.
ceDigi Log file.
muDauSteps Log file.
ceMix Log file.
rootOverlaps Log file.
g4surfaceCheck Log file.
FIXME, TODO TODO (0) FIXME (0) in 2 files
clang-tidy 🔶 0 errors 185 warnings
whitespace check no whitespace errors found

N.B. These results were obtained from a build of this Pull Request at 7749c0d after being merged into the base branch at 38f72f8.

For more information, please check the job page here.
Build artifacts are deleted after 5 days. If this is not desired, select Keep this build forever on the job page.

EventGenerator/src/RPCGun_module.cc Outdated Show resolved Hide resolved
EventGenerator/src/RPCGun_module.cc Outdated Show resolved Hide resolved
EventGenerator/src/PionFilter_module.cc Show resolved Hide resolved
@kutschke
Copy link
Collaborator

Please look through the code and fix other instances of abs to std::abs.

@sophiemiddleton
Copy link
Contributor Author

thanks, I found two other instances, now fixed

@sophiemiddleton
Copy link
Contributor Author

@FNALbuild run build test

@FNALbuild
Copy link
Collaborator

⌛ The following tests have been triggered for bcd4eab: build (Build queue is empty)

@FNALbuild
Copy link
Collaborator

☀️ The build tests passed at bcd4eab.

Test Result Details
test with Command did not list any other PRs to include
merge Merged bcd4eab at 38f72f8
build (prof) Log file. Build time: 08 min 30 sec
ceSimReco Log file.
g4test_03MT Log file.
transportOnly Log file.
POT Log file.
g4study Log file.
cosmicSimReco Log file.
cosmicOffSpill Log file.
ceSteps Log file.
ceDigi Log file.
muDauSteps Log file.
ceMix Log file.
rootOverlaps Log file.
g4surfaceCheck Log file.
FIXME, TODO TODO (0) FIXME (0) in 2 files
clang-tidy 🔶 0 errors 180 warnings
whitespace check no whitespace errors found

N.B. These results were obtained from a build of this Pull Request at bcd4eab after being merged into the base branch at 38f72f8.

For more information, please check the job page here.
Build artifacts are deleted after 5 days. If this is not desired, select Keep this build forever on the job page.

@sophiemiddleton
Copy link
Contributor Author

Any more comments on this, I would like to begin making pion samples this week

Copy link
Collaborator

@kutschke kutschke left a comment

Choose a reason for hiding this comment

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

For a future PR, please split RPCGun_module into a generator, without histograms, and an analyzer that makes histograms. In the very early days we encouraged a single module that does both but we changed our minds some years ago and we want to move everything in the new direction. SInce I was not available to pay attention to PRs for so long, I won't insist on it now.

Added issue #1352 to address this.

@kutschke
Copy link
Collaborator

kutschke commented Oct 4, 2024

@FNALbuild run build test

@FNALbuild
Copy link
Collaborator

⌛ The following tests have been triggered for bcd4eab: build (Build queue is empty)

@FNALbuild
Copy link
Collaborator

☀️ The build tests passed at bcd4eab.

Test Result Details
test with Command did not list any other PRs to include
merge Merged bcd4eab at 5eb44d8
build (prof) Log file. Build time: 04 min 14 sec
ceSimReco Log file.
g4test_03MT Log file.
transportOnly Log file.
POT Log file.
g4study Log file.
cosmicSimReco Log file.
cosmicOffSpill Log file.
ceSteps Log file.
ceDigi Log file.
muDauSteps Log file.
ceMix Log file.
rootOverlaps Log file.
g4surfaceCheck Log file.
FIXME, TODO TODO (0) FIXME (0) in 2 files
clang-tidy 🔶 0 errors 180 warnings
whitespace check no whitespace errors found

N.B. These results were obtained from a build of this Pull Request at bcd4eab after being merged into the base branch at 5eb44d8.

For more information, please check the job page here.
Build artifacts are deleted after 5 days. If this is not desired, select Keep this build forever on the job page.

@kutschke
Copy link
Collaborator

kutschke commented Oct 4, 2024

This PR has no coverage in the nightly validation - as best I can tell it affects only JobConfig/beam/POT_infinitepion.fcl . The oustanding request will be covered in Issue #1352 . Merging now.

@kutschke kutschke merged commit 5694fbe into Mu2e:main Oct 4, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants