colvars icon indicating copy to clipboard operation
colvars copied to clipboard

Better error message when trying to load coordinates from missing files

Open giacomofiorin opened this issue 11 months ago • 3 comments

When loading a file that isn't XYZ the first branch of the conditional below fails, falling back to the PDB reader: https://github.com/Colvars/colvars/blob/8eb35e75c43d6f7e16d8e634e827639097d16368/src/colvarmodule.cpp#L2159-L2171 In GROMACS the resulting error message can be confusing.

It may be clearer to check first if the file is present and readable before discussing its format.

giacomofiorin avatar Mar 05 '24 22:03 giacomofiorin

Couldn't the same purpose be achieved by a more portable test? Namely try to open the file, and if not good() we give up.

Second question: will this play nicely with the input file caching in GROMACS? At mdrun time, the input files may not exist in the filesystem.

jhenin avatar Mar 12 '24 19:03 jhenin

Couldn't the same purpose be achieved by a more portable test? Namely try to open the file, and if not good() we give up.

Second question: will this play nicely with the input file caching in GROMACS? At mdrun time, the input files may not exist in the filesystem.

Both good points. In that case, rather than reinventing the wheel or making the tests more complex for GROMACS, we could perhaps recognize that load_coords() is really intended for PDB files, and make its error messages more explicit that way.

This would also help in the case where people try using a PDB file in NAMD or VMD and then suddenly it doesn't work in GROMACS...

giacomofiorin avatar Mar 12 '24 19:03 giacomofiorin

Both good points. In that case, rather than reinventing the wheel or making the tests more complex for GROMACS, we could perhaps recognize that load_coords() is really intended for PDB files, and make its error messages more explicit that way.

Implemented in #673.

giacomofiorin avatar Mar 14 '24 13:03 giacomofiorin