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

Resolves Iss1568 where LTO build gives warnings about unused variables #1590

Merged
merged 3 commits into from
Feb 11, 2025

Conversation

LiamBrennan-UCSB
Copy link
Contributor

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

What are the issues that this addresses?

This resolves #1568 by removing the unused variables in TrackDeDxMassEstimator.h, TrigScintFirmwareTracker.h, HcalDigiProducer.cxx, and adding override to TrkDeDxMassEstFeatures.h

Check List

  • I successfully compiled ldmx-sw with my developments
  • I ran my developments and the following shows that they are successful.

Liam Brennan and others added 3 commits February 8, 2025 17:24
…DeDxMassEstFeatures. Added ignore statements to variables in TrigScintFirmwareTracker. Removed Verbosity in TrackDeDxMassEstimator.
@LiamBrennan-UCSB
Copy link
Contributor Author

@rodwyer100 Hey Rory, I was told to ping you as I deleted some variables I believe you added. They were causing the LTO build to fail. Did you need these variables? If so I can re-add them and use [[maybe_unused]] instead

@tvami
Copy link
Member

tvami commented Feb 11, 2025

/run-validation

Copy link
Contributor

The validation workflow is running here: https://github.com/LDMX-Software/ldmx-sw/actions/runs/13254346603.

@LiamBrennan-UCSB LiamBrennan-UCSB changed the title Iss1568 lto warnings Resolves Iss1568 where LTO build gives warnings about unused variables Feb 11, 2025
Copy link
Member

@tvami tvami left a comment

Choose a reason for hiding this comment

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

Looks good!

Copy link
Member

@tomeichlersmith tomeichlersmith left a comment

Choose a reason for hiding this comment

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

Looks good to me too :)

@tvami
Copy link
Member

tvami commented Feb 11, 2025

Hey Rory, I was told to ping you as I deleted some variables I believe you added. They were causing the LTO build to fail. Did you need these variables? If so I can re-add them and use [[maybe_unused]] instead

I asked him on slack this is what he wrote

"I suppose they were used by earlier versions of firmware, especially the max depth variable as its hardcoded to be depth 2 now. The other variables (centroids) are only unused afaik because the last step (forming real tracks from the firmware ones) doesn't use those quantities. It calculates centroids from the cluster centroids. I believe its safe to remove them."

@tvami tvami merged commit e6acfb2 into trunk Feb 11, 2025
@tvami tvami deleted the iss1568-LTO-Warnings branch February 11, 2025 22:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Get rid of newly introduced LTO warnings
3 participants