colvars icon indicating copy to clipboard operation
colvars copied to clipboard

Clarify purpose of functions that only handle PDB files anyway

Open giacomofiorin opened this issue 2 years ago • 2 comments

Coordinate loading functions are renamed to reflect that they are specific to the PDB format.

Fixes #667

giacomofiorin avatar Mar 13 '24 15:03 giacomofiorin

A very good change! I'm unsure that it solves all the problems that were raised in #667 though: the case of a missing file, grompp vs. mdrun?

jhenin avatar Mar 13 '24 16:03 jhenin

A very good change! I'm unsure that it solves all the problems that were raised in #667 though: the case of a missing file, grompp vs. mdrun?

It wouldn't. However, it would no longer print the generic message "loading atomic coordinates from a file is currently not implemented", but rather one specific to the PDB format.

Then if the XYZ file is missing/unreadable, we get the same error message as with any other file.

giacomofiorin avatar Mar 13 '24 16:03 giacomofiorin

Then I propose merging this, but not closing #667 just yet.

jhenin avatar Mar 13 '24 18:03 jhenin

I edited the description to reflect your suggestion, but I'm not sure I understand why.

If we can't use std::filesystem::exists() and you propose instead to try open a file to catch a generic read error, that would not be different from the current behavior for a XYZ file in GROMACS. But for a PDB, we'd want the "not implemented" error to come out regardless of whether the file is unreadable.

giacomofiorin avatar Mar 13 '24 22:03 giacomofiorin