search

Colvars / colvars

Collective variables library for molecular simulation and analysis programs
http://colvars.github.io/
GNU Lesser General Public License v3.0
196 stars 56 forks source link

Clarify purpose of functions that only handle PDB files anyway #673

Closed giacomofiorin closed 3 months ago

giacomofiorin commented 3 months ago

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

jhenin commented 3 months ago

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?

giacomofiorin commented 3 months ago

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.

jhenin commented 3 months ago

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

giacomofiorin commented 3 months ago

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.