MB patched to Heme is failing when compiled in DEBUG mode
GregorySchwing opened this issue · 9 comments
Describe the bug
Dev branch all the way back to the HVAP fix, so far, cannot parse Myoglobin patched to heme, but ONLY IN DEBUG mode. In RELEASE MODE IT WORKS. This is highly unusual, and it's unlikely I'll find a commit which could handle this system.
It's possible the compiler is fixing a memory access problem, which the debug mode turns off. Or a misplaced NDEBUG. Hopefully, this isn't a bug in the PSF parser, but it could be.
Compiling with the following process:
./metamake.sh -g GCMC
Produces a binary which begins the simulation then runs to completion.
Compiling with the following process:
./metamake.sh -gd GCMC
Results in this error message:
GOMC_CPU_GCMC: /home/greg/Desktop/GOMC/src/cbmc/DCLinkedCycle.cpp:84: cbmc::DCLinkedCycle::DCLinkedCycle(cbmc::DCData*, const mol_setup::MolKind&, std::vector, uint, uint): Assertion `match != dihs.end()' failed.
Aborted (core dumped)
To Reproduce
./rel_bin/GOMC_CPU_GCMC in_GCMC_NVT.conf > release.log
./deb_bin/GOMC_CPU_GCMC in_GCMC_NVT.conf > debug.log
Expected behavior
Input files should run in release and fail in debug.
test.zip
Please complete the following information:
- OS: Ubuntu
- Ensemble: GCMC
- Code version - dev
There are only a few pull requests merged since that one. Can you narrow it down further? The very next pull request that was merged was my removal of the MIE_INT_ONLY compile-time flag. I took another look at that patch. Since that flag wasn't being used, I can't imagine that would cause this problem. Note that there was a conflict with System.cpp on the debug-info patch, so please ensure that conflict was resolved properly.
There are only a few pull requests merged since that one. Can you narrow it down further? The very next pull request that was merged was my removal of the MIE_INT_ONLY compile-time flag. I took another look at that patch. Since that flag wasn't being used, I can't imagine that would cause this problem. Note that there was a conflict with System.cpp on the debug-info patch, so please ensure that conflict was resolved properly.
Update: It has nothing to do with your patches. It has been hiding all along. This is a bug in representing this molecule in GOMC. Unfortunately, GOMC only complains if compiled in Debug mode. It is either the behavior is going unnoticed in release mode (bad) or the debug compile flag is causing the bug (less bad).
I don't think we make extensive use of NDEBUG statements where we build molecules or handle input. We can use address sanitizer to try to track down any out of bounds memory accesses. We could also be using some uninitialized memory, which could have different values when compiled with and without debug flags.
I have attached an two unsolvated MB (pdb/psf) pairs one with AUTO ANGLES DIHEDRALS and one without. The one with all the angles dihedrals should run in GOMC, while the latter should reproduce the error this issue opened. If this is the case, this issue can be resolved as a input file problem.
MB.zip
It turns out that this problem is not caused by running in DEBUG mode. The assert() statement is not executed in release mode, but the error still occurs: in other words, it isn't finding a match. Will test the new example to see if the problem is in how the cycles are being built or is related to the input file.
Can you provide a config file to run? I tried editing one, but it seems that I'm not sure what changes are needed.
Added a config file and parameter file generated using the FFToolkit in VMD for the missing dihedrals.
The conundrum is in release mode both FEW dihedrals and ALL dihedrals run.
In Debug mode, there is an assertion that all the dihedrals be present, and thus it terminates on an assertion.
This is the error, since I resolve it by generating all the dihedrals. Therefore, it's not a problem with the cycle detection or parser.
All the dihedrals don't have to be present for GOMC to run using crankshaft moves, but after the first regrowth move is accepted the FEW dihedrals segfaults on the next move.
I'm closing the issue, since I resolved the error by generating all the dihedrals, and demonstrating the code segfaults on regrowth moves if you don't have all the dihedrals defined.
In conclusion:
To currently run GOMC all dihedrals must be defined, and there's no bugs in parsing complex molecules as long as all DIHEDRALS are present.
Should we update the code to ensure that all the dihedrals are present if regrowth moves are enabled? It seems like that's a better option than generating a segfault but perhaps that isn't feasible.
Right, I have run into this before when I have a malformed PSF that does not include required dihedrals. The code will segfault. It would be nice to have some error checking for this.