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 64-bit time step counters #229

Closed
joaander opened this issue Feb 23, 2017 · 5 comments · Fixed by #894
Closed

Use 64-bit time step counters #229

joaander opened this issue Feb 23, 2017 · 5 comments · Fixed by #894
Assignees
Labels
breaking Changes that will break API. enhancement New feature or request
Milestone

Comments

@joaander
Copy link
Member

Original report by me.


  • There are many "unsinged int timestep" and "unsigned int m_timestep" phrases in the code that can be bulk replaced.
  • System.h and System.cc will need a read-through to find all instances where it stores a timestep like variable in an unsinged int (even temporarily in a delta).
  • Special care must be taken in DPD and anywhere else that uses the current timestep in a Saru RNG - the easiest thing to do is cast out the lower 32 bits of the number and use that for the seed, though that will make RNGs repeat every ~4 billion steps. Alternately, one could mix the higher 32 bits into one of the other seed entries.
  • the gsd file format already stores time step as a uint64_t, so no changes are needed there.
@joaander
Copy link
Member Author

  • any .cu file that uses timestep has several steps where timestep is copied (grep "unsigned int _timestep" finds some of these)

@joaander
Copy link
Member Author

This would be nice to have, but it is not a priority. Realistically, no one has time to complete it.

If this is important to you, feel free to reopen the issue and work with me to make the necessary changes to hoomd.

@joaander joaander added major enhancement New feature or request labels Feb 12, 2019
@joaander joaander added this to the v2.3 milestone Feb 18, 2019
@joaander joaander modified the milestones: v2.3, v3.0 Nov 1, 2019
@joaander joaander reopened this Nov 1, 2019
@joaander
Copy link
Member Author

We will implement this in v3.0.

Questions: The DCD file format cannot represent 64-bit timestep counters. Should we remove DCD, let the counter wrap around, or error when attempting to write an invalid step to DCD?

Aside: DCD can also only represent fixed periods and the new Trigger API in HOOMD will make it even easier for users to use non-fixed period outputs.

@joaander joaander added the breaking Changes that will break API. label Nov 14, 2019
@mphoward
Copy link
Collaborator

I think we should keep supporting DCD because users of other simulation packages casually trying or transitioning to HOOMD might prefer having this file format for their other tools.

I would error out if the timestep exceeds the DCD spec. This can be clearly documented, and the user should decide how they want to handle longer runs (reset the step, create a new file, etc.)

@vyasr
Copy link
Contributor

vyasr commented Nov 16, 2019

I agree with @mphoward, removing DCD would probably alienate significant portions of our user base. There's no need to improve its support, just to keep it where it is now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Changes that will break API. enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants