lorenzo-rovigatti / tacoxDNA

A collection of tools for DNA modelling
http://tacoxdna.sissa.it/
GNU General Public License v3.0
17 stars 20 forks source link

Read NUMBER OF ATOMS and BOX BOUNDS at every checkpoint #31

Closed ohenrich closed 3 years ago

ohenrich commented 3 years ago

Hi Lorenzo,

The script threw an error when the first timestep in the trajectory file wasn't t=0 as NUMBER OF ATOMS and BOX BOUND weren't set then.

I modified the code to read NUMBER OF ATOMS and BOX BOUNDS at every timestep. There is virtually no impact on performance and conversion of trajectory files is now also possible when the first timestep is not zero.

Pull request for python2 comes in a moment.

Cheers, O

lorenzo-rovigatti commented 3 years ago

Hey Oli, thanks for the PR. I think there is only an issue with this: the topology is printed just once per trajectory. If the number of nucleotides changes we will have incompatible topology/conf files. How do we take this into account? Do we issue a warning (by checking whether the new natoms is different from the old one)?

ohenrich commented 3 years ago

I see. But is the number of nucleotides actually changing? It never does in LAMMPS so far, so issuing a warning would work for the time being. What do you think?

lorenzo-rovigatti commented 3 years ago

I see. But is the number of nucleotides actually changing? It never does in LAMMPS so far, so issuing a warning would work for the time being. What do you think?

I think that would be indeed enough.

ohenrich commented 3 years ago

Where would you put the warning exactly? Basically always printed in the constructor? I can modify the code and add this to the PR, but probably only after 3PM UK time.

lorenzo-rovigatti commented 3 years ago

I think something like this would suffice (I write it without having tested it but you get my point I think):

natoms = int(lmptrj.readline())
if natoms != conf.natoms:
    print("WARNING: A configuration stored in the trajectory file contains a number of nucleotides %d that differs from the datafile one, %d" % (natoms, conf.natoms), file=sys.stderr)

what do you think?

ohenrich commented 3 years ago

OK thanks. I'll put this in and test it.

ohenrich commented 3 years ago

Hi Lorenzo,

I pushed these changes, but made them an ERROR message as it doesn't work unless data file and trajectory file specify the same number of nucleotides.

Let me know if this is OK or what else needs to be changed. I'll write later today as I have another question re our oxDNA3 branch and how to best avoid code duplication further down the line.

CU O

lorenzo-rovigatti commented 3 years ago

Hey Oli, thanks. If the code is not supposed to work if the numbers are different, shouldn't we exit in addition to printing out the error message?

ohenrich commented 3 years ago

Yes, I think we should!