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

Move msgs in EcalProcessFilter to logging system #1277

Merged
merged 1 commit into from
Apr 10, 2024
Merged

Conversation

tvami
Copy link
Member

@tvami tvami commented Apr 8, 2024

I am updating ldmx-sw, here are the details.

What are the issues that this addresses?

This resolves #1250

Check List

  • I successfully compiled ldmx-sw with my developments

  • I ran my developments and the following shows that they are successful.

Running

ldmx fire ldmx-sw/.github/validation_samples/ecal_pn/config.py

Now the

[ EcalProcessFilter ]: (event nb) Brem photon produced (some number of) particle via biasWrapper(photonNuclear) process.

are not printed out by default.

  • I attached any sub-module related changes to this PR.

N/A

@tvami
Copy link
Member Author

tvami commented Apr 8, 2024

I also expect the log diff tests to fail with this

@tvami tvami self-assigned this Apr 8, 2024
Copy link
Contributor

@bryngemark bryngemark left a comment

Choose a reason for hiding this comment

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

Really great, thanks for following up on this @tvami!

@bryngemark
Copy link
Contributor

indeed the log diff is (except a few time stamps from configuration printouts) an ecal printout for each event. @tomeichlersmith @EinarElen thoughts on this, should the CI use logs at debug level to catch any differences?

@EinarElen
Copy link
Contributor

Seems reasonable to me!

@tvami
Copy link
Member Author

tvami commented Apr 9, 2024

should the CI use logs at debug level to catch any differences?

I agree with this! It should be for all the tests, so beyond this PR, maybe we should just made its own issue for this?

@tvami tvami mentioned this pull request Apr 9, 2024
3 tasks
@tomeichlersmith tomeichlersmith merged commit 67a948a into trunk Apr 10, 2024
6 of 7 checks passed
@tomeichlersmith tomeichlersmith deleted the iss1250 branch April 10, 2024 14:16
@tvami
Copy link
Member Author

tvami commented Apr 10, 2024

should the CI use logs at debug level to catch any differences?

I agree with this! It should be for all the tests, so beyond this PR, maybe we should just made its own issue for this?

For reference: #1280

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.

EcalPN is too verbose
4 participants