GOMC-WSU / GOMC

GOMC - GPU Optimized Monte Carlo is a parallel molecular simulation code designed for high-performance simulation of large systems
https://gomc-wsu.org
MIT License
76 stars 36 forks source link

Better error messages when failing on read #231

Open jpotoff opened 4 years ago

jpotoff commented 4 years ago

We've made a lot of improvements in how we handle errors on reading input files, but we still need to get better. This simulation returns an error:

Reading from CHARMM-Style parameter file: ../input/par_trappe_pso.inp Reading BONDS parameters. Reading ANGLES parameters. Reading DIHEDRALS parameters. Error: Incomplete Dihedral parameters were found in parameter file!

This is better than a segfault, but it still requires one check every dihedral, which is somewhat annoying. Better for it to flag exactly which dihedral is not correctly defined.

input.zip

jpotoff commented 4 years ago

Changing this to a bug. The same files when read in a prior version of 2.60 development work fine.

jpotoff commented 4 years ago

This problem seems to only impact grand canonical and Gibbs ensemble Monte Carlo simulations (where we have two boxes).

GregorySchwing commented 4 years ago

This is the same error I got today when trying to read a Charmm file into a mie ff.
ParaTypeMie true Parameters ./par_Charmm.par It's possible both aren't setting isCharm = true.

jpotoff commented 4 years ago

@GregorySchwing that's a good call. That's exactly the problem. I was trying to load a Mie force field file with ParaTypeCHARMM=true. However, the error I got was about dihedral angles, which wasn't the actual problem. And it only appears for GCMC and GEMC simulations. So we need to think about how to better handle error checking of parameter files.

LSchwiebert commented 4 years ago

It looks like FFSetup.cpp, line 291, which is function Dihedral::Read, we always generate an error if the dihedral angle data isn't found in the input file. Not sure why we would get this error for other reasons. Sounds like we need a more sophisticated test.

jpotoff commented 4 years ago

Not sure of the order we are reading and flagging errors. But, in the Mie format parameter files we have a section labeled "NONBONDED_MIE." The CHARMM style parameter files have a section header "NONBONDED." So at a minimum we should send a warning to the user if they have selected a certain parameter file type in the input file (*.conf), but are actually attempting to read the other one.

GregorySchwing commented 4 years ago

I implemented the error checking you asked for, but I can't get the simulation to run for this reason:

Error: MEMC move cannot be applied to pure systems or systems, where only one molecule type is allowed to be swapped.

Any ideas? We had this error checking already, but it was after all the headers are read. We require the next header to be valid for the previous header to finish successfully. Dihedrals is previous to Nonbonded. Hence, why Dihedrals was where the error message was printed from.

GregorySchwing commented 4 years ago

I deleted the MEMC move in config, and the simulation runs. I can open a pull request for this error checking. Essentially, we need map entries with all the possible headers, even if we never use them. We use to only add string keys for the appropriate forcefield. I removed this constraint, and check afterwards if we initialized values for keys that don't fit the forcefield.

GregorySchwing commented 4 years ago

233

jpotoff commented 4 years ago

I implemented the error checking you asked for, but I can't get the simulation to run for this reason:

Error: MEMC move cannot be applied to pure systems or systems, where only one molecule type is allowed to be swapped.

Any ideas? We had this error checking already, but it was after all the headers are read. We require the next header to be valid for the previous header to finish successfully. Dihedrals is previous to Nonbonded. Hence, why Dihedrals was where the error message was printed from.

That is an actual error. I should have deleted that move out of the config file, but forgot. I'm surprised it wasn't caught before. But I think this highlights some issues with our error checking in terms of where it occurs in the process.