-
Notifications
You must be signed in to change notification settings - Fork 647
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
Can't open NetCDF trajectories created by cpptraj #4073
Comments
This is a complicated one, and I think may come down to an interpretation of how the NetCDF format is defined. For context the NetCDF trajectory convention is here: https://ambermd.org/netcdf/nctraj.xhtml As per the convention (when talking about the
The word However, we also find above:
Whilst the first half of this statement would make it seem that As such I would have expected cpptraj to write the I think this is probably a question for @drroe, as both an author of that format spec and the lead dev of cpptraj - what should be the expected behaviour here? |
I really hope that CPPTRAJ will eventually adhere to AMBER's own conventions :-) In the meantime, maybe a workaround to be able to use CPPTRAJ-created trajectories could be useful, as if and when CPPTRAJ will be "fixed", the already generated trajectories will still be unusable, what do you think? |
I'll be honest, I'm not particularly keen to start adding exceptions for things that break format spec (not saying that this is a break in format spec, I would say this ultimately falls to the [edit: spec] author's discretion). The issue you face here is that this is the type of thing where you start introducing lots of spagetthi in your code to try to adhere to arbitrary problems elsewhere, and then very quickly you have a piece of code that's completely unmaintainble. A prime example of this is our PDB parser, which has had to do a lot of workarounds for things like the CHARMM format variants, and ultimately has become an extremely slow & bloated reader. Should this be a bug in cpptraj (again, please don't take this as a claim that it is), I would expect cpptraj itself to be able to offer a means of recovering "fixed" trajectories. |
I understand, I'll just use my patched version until CPPTRAJ's authors will "fix" it then! |
"Shall" here means that if there is simulation time data present, the variable should be defined. This is the case with molecular dynamics trajectory data, but may not always be the case; for example the It shouldn't be too difficult to change the code to make the time variable optional since the NetCDF format is so orthogonal. Just do a
As far as I can tell in this instance and elsewhere, CPPTRAJ is adhering to the Amber NetCDF standard as written. If you still feel this is not the case please open an issue on the CPPTRAJ GitHub site so it stays on my radar. Thanks, and hope this helps. |
Thanks so much for the explanation @drroe, this is super useful! @DrDomenicoMarson this does mean that we should try to fix for this case. However, whilst the proposed solution you laid out in your initial comment isn't bad, it might not be completely the right answer either. Because these are non-temporal frames, setting the time to the frame id is probably not the ideal solution, instead we should consider setting everything to single value (probably On top of that we probably should just change how we derive the number of frames when |
The
mda.coordinates.TRJ.NCDFReader
expect an Amber-generated NetCDF trajectory to havetime
information in thevariables
object of the NetCDF file.This is true for NetCDF files created by AMBER's MD engine, but that's not the case for files generated by AMBER's CPPTRAJ.
If I try to read a NetCDF file created by CPPTRAJ, I end up with the error message raised in line 532 of
mda.coordinates.TRJ.NCDFReader
, as the keytime
doesn't exist in the trajectoryvariables
.The assumption of the existence of the
time
variable is made in other parts of the code as well.It would be great to not have this assumption in place.
I edited the code in my fork, and I made it to read successfully NetCDF files generated by CPPTRAJ, but I don't know if how I made it could break something else in the workflow.
Simply, anywhere the code expected to read time information from the
time
variable, I replaced this information with the current frame index (fordt
there is already a default behavior that usesdt=1ps
if not read from the trajectory).There is a problem with this implementation, although I don't know if this is a big problem after all:
if I create a mda.Universe starting from a list of NetCDF files, the resulting universe does have not an incrementing
time
, as each concatenated trajectory file restarts itstime
from 0for example, I have 5 trajectories of 100 frames, if I create an Universe from them and then call
I see values of 0 to 99 repeated 5 times, instead of 0 to 499.
If you want I can create a pull-request with the small modifications I made, if this behaviour is not expected to break some other functionalities of mda, as I just started using it and I'm not an expert at all!
The text was updated successfully, but these errors were encountered: