Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better error message when trying to load coordinates from missing files #667

Open
giacomofiorin opened this issue Mar 5, 2024 · 3 comments
Assignees

Comments

@giacomofiorin
Copy link
Member

When loading a file that isn't XYZ the first branch of the conditional below fails, falling back to the PDB reader:

colvars/src/colvarmodule.cpp

Lines 2159 to 2171 in 8eb35e7

if (colvarparse::to_lower_cppstr(ext) == std::string(".xyz")) {
if (pdb_field.size() > 0) {
return cvm::error("Error: PDB column may not be specified "
"for XYZ coordinate files.\n", COLVARS_INPUT_ERROR);
}
// For XYZ files, use internal parser
error_code |= cvm::main()->load_coords_xyz(file_name, &sorted_pos, atoms);
} else {
// Otherwise, call proxy function for PDB
error_code |= proxy->load_coords(file_name,
sorted_pos, atoms->sorted_ids(),
pdb_field, pdb_field_value);
}

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.

@jhenin
Copy link
Member

jhenin commented Mar 12, 2024

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.

@giacomofiorin
Copy link
Member Author

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
Copy link
Member Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants