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

Use 64bit integers to store prev timestep #640

Merged
merged 1 commit into from
Dec 5, 2023
Merged

Conversation

jhenin
Copy link
Member

@jhenin jhenin commented Dec 5, 2023

Should fix #639

@giacomofiorin
Copy link
Member

giacomofiorin commented Dec 5, 2023

Looks good to me!

For reference:

  • NAMD was not affected by the bug because size_t is already 64-bits (only unsigned).
  • LAMMPS was affected, although it's unclear how frequently: jobs of > 2^32 steps are very rare
  • patched GROMACS versions were affected.

What about GROMACS 2024?

@jhenin
Copy link
Member Author

jhenin commented Dec 5, 2023

In GROMACS 2024, Colvars gets the current time step explicitly through the ForceProvider API, so not affected.

@jhenin
Copy link
Member Author

jhenin commented Dec 5, 2023

Does the standard specify that size_t is 64 bit? Maybe in practice only?
https://en.cppreference.com/w/c/types/size_t

@jhenin jhenin marked this pull request as ready for review December 5, 2023 19:07
@jhenin jhenin merged commit 4bef804 into master Dec 5, 2023
17 checks passed
@giacomofiorin
Copy link
Member

Does the standard specify that size_t is 64 bit? Maybe in practice only? https://en.cppreference.com/w/c/types/size_t

I believe you'd have to go to a 32-bit architecture to find a narrower type for size_t.

@giacomofiorin giacomofiorin deleted the fix_prev_step_overflow branch December 5, 2023 19:10
@giacomofiorin giacomofiorin added the bugfix To be used only in PRs label Dec 5, 2023
@giacomofiorin giacomofiorin mentioned this pull request Aug 5, 2024
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix To be used only in PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ABF histogram overflow shortly after 2 microseconds
3 participants