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

Check that cythonisation of XDR reader didn't introduce C int overflow bug similar to that in #4039 #4053

Closed
hmacdope opened this issue Mar 6, 2023 · 4 comments

Comments

@hmacdope
Copy link
Member

hmacdope commented Mar 6, 2023

Cythonisation of the DCD reader in #3888 resulted in an int overflow bug as detailed in #4039 and fixed in #4084.

Given I did something similar for the XDR reader in #3892, we should check that no similar bugs were introduced.

@hmacdope hmacdope self-assigned this Mar 6, 2023
@IAlibay
Copy link
Member

IAlibay commented Mar 24, 2023

I've tested this in two ways:

  1. Create a ~ 16 GB XTC trajectory of 100000 frames as per Error when loading multiple large DCD trajectories #4039 using the TPR system.
  2. Create a small 10000000 frames (~ 1.2 GB) XTC trajectory for a subset of the TPR system.

And then trying to load both to memory. In both cases I didn't encounter any issues.

@hmacdope
Copy link
Member Author

@ianmkenney to test quickly on a large XTC privately and then good to go

@ianmkenney
Copy link
Member

I iterated over a TRR file:

04:18 PM >>> du -sh production.trr 
292G    production.trr

and I didn't get a segfault

In [3]: u = mda.Universe("top.dms", "production.trr")

In [4]: from tqdm import tqdm

In [5]: for ts in tqdm(u.trajectory):
   ...:     pass
   ...: 
100%|████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████| 191666/191666 [45:15<00:00, 70.58it/s]

In [6]: mda.__version__
Out[6]: '2.5.0-dev0'

In [7]: u.atoms
Out[7]: <AtomGroup with 136294 atoms>

Hope that helps!

@hmacdope
Copy link
Member Author

With that I think we can close. Thanks @ianmkenney!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants