Skip to content

Cleanup and refactor LoadWCSim#372

Open
atcsutton wants to merge 4 commits intoANNIEsoft:Applicationfrom
atcsutton:feature/LoadWCSimCleanup
Open

Cleanup and refactor LoadWCSim#372
atcsutton wants to merge 4 commits intoANNIEsoft:Applicationfrom
atcsutton:feature/LoadWCSimCleanup

Conversation

@atcsutton
Copy link
Contributor

Describe your changes

Title says it all. LoadWCSim had a lot of copy pasta and this fixes it. This makes it easier to read and make modifications later. I also took the opportunity to adjust layout and formatting to my preferences.

Checklist before submitting your PR

  • This PR implements a single change (one new/modified Tool, or a set of changes to implement one new/modified feature)
  • This PR alters the minimum number of files to affect this change
  • If this PR includes a new Tool, a README and minimal demonstration ToolChain is provided
  • If a new Tool/ToolChain requires model or configuration files, their paths are not hard-coded, and means of generating those files is described in the readme, with examples provided on /pnfs/annie/persistent
  • For every new usage, there is a reason the data must be on the heap
  • For every new there is a delete, unless I explicitly know why (e.g. ROOT or a BoostStore takes ownership)

I didn't create anything new, mainly just rearranged, so the last four boxes are unchecked.

Additional Material

I loaded and saved a single WCSim file. Then used LoadANNIEEvent and PrintANNIEEvent to verify that the resulting files are the same (or nearly so). The only "unintended" difference I found was a minor change in the floating point precision of MCParticle::trackLength (eg. 0.563564 → 0.563563) that will have no impact.

… fix to the PlotWaveforms Makefile to fix a build error
@jminock
Copy link
Collaborator

jminock commented Feb 13, 2026

Haven't went over in detail yet, but I noticed the workflow build failed. It seems unrelated to the PR, and I haven't found anything in the PR that would break the build. Sometimes this just happens.

Copy link
Collaborator

@jminock jminock 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! Thank you for cleaning this up!

minor comment changes just to trigger CI with updated workflow.
another tweak to retrigger CI workflow with updated action
yet another workflow attempt
@marc1uk
Copy link
Collaborator

marc1uk commented Feb 15, 2026

alright, the first thing i can't help but notice (because the git diff highlights the entire file as having changed) is that the whitespace has been switched from consistent tab-intenting to a combination of tabs and spaces.
For my sanity, can we not do this? Please? 🥲

MCFile = WCSimEntry->GetCurrentFile()->GetName();

// Clean slate
std::cout << TDCData->size() << std::endl;
Copy link
Collaborator

Choose a reason for hiding this comment

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

omit couts, especially as this is probably misleading to print the number of hits just priot to clearing and re-populating them?

@marc1uk
Copy link
Collaborator

marc1uk commented Feb 15, 2026

Alright, finally got the CI approval, looks fine to me, i agree it's more readable like this. 👍

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.

3 participants