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

Split trajectory frames into chunks #602

Merged
merged 9 commits into from
Nov 20, 2024
Merged

Split trajectory frames into chunks #602

merged 9 commits into from
Nov 20, 2024

Conversation

MBartkowiakSTFC
Copy link
Collaborator

Description of work
Trajectory arrays such as positions, velocities and charges are now split into fixed-size chunks within a single time step.

closes #596

Note: the changes in this PR will break the scripts in MDANSE-Examples. The scripts should be updated around the time of the next release.

Fixes

  1. Changed chunking in TrajectoryWriter. The arrays are padded to ensure that their size is a multiple of the chunk size.
  2. Added chunk size as a parameter to OutputTrajectoryConfigurator.
  3. Added chunk size to OutputTrajectoryWidget.
  4. Replaced test trajectories in unit tests with chunked ones.
  5. Updated test inputs to inlcude the new parameter.

To test
Convert a trajectory using the protos branch. Run Dynamic Coherent/Incoherent Structure Factor analysis on this trajectory and time both runs.
Repeat the same steps using this branch. Compare the results and the timing.
Expected results: DCSF will slow down by ca. 5%, DISF will speed up by ca. 25%, the results should not change.

Copy link
Collaborator

@ChiCheng45 ChiCheng45 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. Calculations were run on a single core.

DISF time reduced from 4.5 mins to 1 min. (500 frames, 2000 Atoms and 128 chunk)
DCSF times were similar 25s vs 20s. (2000 frames, 2000 Atoms and 128 chunk)

This is definitely an improvement for trajectories with a larger number of atoms and I find the same results with the new and old chunking schemes. I think there is probably more we could do to improve performances, we should look at this again in the future.

@ChiCheng45 ChiCheng45 merged commit 84fec99 into protos Nov 20, 2024
28 checks passed
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.

[ENHANCEMENT] Improve trajectory chunking
2 participants