GOMC
GOMC copied to clipboard
(#523) final energy output omission correction
(@LSchwiebert Hopefully this is what you meant)
This PR adjusts the Clock::CheckTime() condition to handle cases where stepDelta >= stepsPerOut or step == lastStep, preventing any missed final energy output and unifying the timing path.
Fixes #523
It seems I added my comment to the issue, not your PR. After thinking about it some more, we don't really need the number of steps printed at the end. The current code keeps us from printing it at the last step and that's fine as is. What we need is to print the energies after the final step. Here is an example, where we run for 20,500 steps. We want the same output of energies that we see after 10,000 and 20,000 steps. So, we need to call this function when we are at the last step, but also checking to make sure we don't accidentally call it twice, such as adding this as a condition or in an else if() block.
@LSchwiebert Ah, I see. I misunderstood this then. Before I adjust the patch maybe I can run this by you first to make sure I'm on the right track.
In OutputAbstracts.h we have...
https://github.com/GOMC-WSU/GOMC/blob/0d5faa49912cff5fab6a6ab242a37b5ed8257edf/src/OutputAbstracts.h#L51-L58
If I read this correctly, this condition only triggers when we're in the first step, when (step + 1) is divisible by stepsPerOut and then when forceOutput is set.
This logic never actually checks for that final step, so if lastStep + 1 is not divisible by stepsPerOut, then DoOutput(lastStep never runs, right?
Yes, that's where you want to make the change. I think if you can figure out something related to the totSteps parameter, you should be all set. The main issue to be aware of is that the simulation may not start at step 0, although I'm not sure whether that has anything to do with this variable.
Best,
Professor Schwiebert
P.S. I think we don't want to change the Clock.h file so you can revert that and not include that in your PR.
--
Loren Schwiebert, PhD Professor, Dept. of Computer Science
@.*** Wayne State University (313) 577-5474
Disclaimer: Opinions expressed are mine and not necessarily shared by WSU.
From: akrm al-hakimi @.> Sent: Sunday, November 16, 2025 8:04 PM To: GOMC-WSU/GOMC @.> Cc: Subscribed @.***> Subject: Re: [GOMC-WSU/GOMC] (#523) final energy output omission correction (PR #543)
[EXTERNAL] [https://avatars.githubusercontent.com/u/111914307?s=20&v=4]cachebag left a comment (GOMC-WSU/GOMC#543)https://github.com/GOMC-WSU/GOMC/pull/543#issuecomment-3539578021
@LSchwieberthttps://github.com/LSchwiebert Ah, I see. I misunderstood this then. Before I adjust the patch maybe I can run this by you first to make sure I'm on the right track.
In OutputAbstracts.h we have...
https://github.com/GOMC-WSU/GOMC/blob/0d5faa49912cff5fab6a6ab242a37b5ed8257edf/src/OutputAbstracts.h#L51-L58
If I read this correctly, this condition only triggers when we're in the first step, when (step + 1) is divisible by stepsPerOut and then when forceOutput is set.
This logic never actually checks for that final step, so if lastStep + 1 is not divisible by stepsPerOut, then DoOutput(lastStep never runs, right?
— Reply to this email directly, view it on GitHubhttps://github.com/GOMC-WSU/GOMC/pull/543#issuecomment-3539578021, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ACZAGWEXBK6R6XL22RHP7FL35ENLFAVCNFSM6AAAAACLVW3EYKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTKMZZGU3TQMBSGE. You are receiving this because you are subscribed to this thread.Message ID: @.***>