Fix units (for good) - Githubissues

ceriottm commented 7 years ago

Let's face it. The latest round of IO/units changes has been a clusterf**k. @ipoltavskyi found it was slowing down things too much, @mahrossi and @venkatkapil24 reported that the units specification in the input was confusing and leading to more problems than it fixed. So here is another try at getting this right. I hope y'all can contribute to making sure we don't have to touch this boring part of the code in the next year at least. Main concepts are:

one should be able to specify units both in the input and in the structure file. if no units are specified in the input, i-PI tries to guess from the comment line, otherwise assumes atomic units, or angstrom for PDB.
if units are specified in both places, and match, conversion happens just once. if they mismatch, an error is raised.
conversion happens in the IO functions as far as possible. this might also be a prequel to a more flexible output infrastructure, but for the moment main change is trajectory outputting passes around indication of units, but the conversion happens in print_file
read_file_raw (and possibly print_file_raw?) are added to access the positions and labels with no conversion and no creation of i-pi objects

mahrossi commented 7 years ago

Hello: what I read and what I tested works as intended. I'm asking for the documentation anyways. I think that one big issue with all the previous infrastructures is that nothing was properly documented. In fact we need to look carefully at the documentation, but we can start small with this one. How to best do it?

ceriottm commented 7 years ago

Excellent suggestion, can you take care of that? I think currently the discussion of units is scattered in different places. Perhaps it might be nice to have a paragraph in the manually-written part of the docs that covers the philosophy behind units in i-PI

On 20 June 2017 at 15:56, Mariana Rossi notifications@github.com wrote:

Hello: what I read and what I tested works as intended. I'm asking for the documentation anyways. I think that one big issue with all the previous infrastructures is that nothing was properly documented. In fact we need to look carefully at the documentation, but we can start small with this one. How to best do it?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/cosmo-epfl/i-pi-dev/pull/188#issuecomment-309761984, or mute the thread https://github.com/notifications/unsubscribe-auth/ABESZ1mc1ZRwjw491TVV2hLvEfYg1TWwks5sF8-lgaJpZM4N9zil .

mahrossi commented 7 years ago

Yes, this was my next question: So we just put it in the manually written part of the manual? If so I'll just do it in this branch.

ceriottm commented 7 years ago

@OndrejMarsalek and @ipoltavskyi I'd like to hear your opinion, and -- possibly -- to have your help making sure that all post-processing tools still work with this revised infrastructure

ipoltavskyi commented 7 years ago

Dear Michele,

My scripts should be modified for the new infrastructure. I will do it ASAP. Also I would add some warning when PDB format is used for output. The actual coordinates may not match the allowed digits in this format, which will lead to a mess in the trajectory files. An example is qtip water in the examples folder. If you run this calculations you will find that the water molecules go away from the original box and may have rather large coordinates. PDB output is useless in this case.

Best wishes Igor