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

data division only works if len(trajectory)%n_parts==0 #159

Closed
v9lgraf opened this issue Oct 11, 2023 · 2 comments · Fixed by #162
Closed

data division only works if len(trajectory)%n_parts==0 #159

v9lgraf opened this issue Oct 11, 2023 · 2 comments · Fixed by #162
Assignees

Comments

@v9lgraf
Copy link

v9lgraf commented Oct 11, 2023

sites = SitesData(
    structure=s,
    trajectory=trajectory,
    floating_specie='Li',n_parts=10
)

Executing the code above throws an error.

This is resolved by instead running:

sites = SitesData(
    structure=s,
    trajectory=trajectory,
    floating_specie='Li',n_parts=6
)

Note that len(trajectory) == 12378. The error is resolved because 12378%6 == 0.

12378%10 != 0 that's why the first code snippet throws an error

This took a while to figure out though and is not robust. This error should be anticipated. Often the MD step number is not divisible by 10

One idea to solve this is clipping some of the final frames from the trajectory so that len(trajectory)%10==0. That would at most mean the loss of 0.18 ps of simulation, that is fine considering ~100 ps are run typically. However, when such clipping occurs a warning message should be displayed saying something like"In the course of the data analysis your simulation time was shortened by 0.xx ps"

@v1kko
Copy link
Contributor

v1kko commented Oct 11, 2023

Yes I agree, dropping some timesteps to make it divisible by n_parts sounds like the way to implement this.
Together with a warning message, and perhaps even a toggle to turn the warning message off: no_warn_truncate=False|True as a keyword to SitesData.

what do you think @stefsmeets ?

@stefsmeets
Copy link
Contributor

Another option is to have uneven parts, e.g. for 38 with n_parts=4: 10, 10, 9, 9

@v1kko v1kko self-assigned this Oct 12, 2023
@v1kko v1kko linked a pull request Oct 12, 2023 that will close this issue
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 a pull request may close this issue.

3 participants