search

Q2MM / q2mm

Quantum to Molecular Mechanics (Q2MM)
MIT License
24 stars 9 forks source link

unknown angle #29

Closed peonor closed 8 years ago

peonor commented 8 years ago

Yet another... I get a crash when torsions to be optimized contain angles which are not optimized. You can see the error message here: tmp.txt It's crashing on the fourth structure in the following mmo file: tmp_mmo.txt As you can see, the angle value is in there, but it's not marked with OPT...

ericchansen commented 8 years ago

Eugh the torsions.

The whole check we use is a temporary fix in the 1st place. It will fail in certain cases.

Ex.) Reference angle is 176 degrees and FF angle is 174 degrees. The FF data will have one more data point than the reference data.

That being said, making another change to the code to make it work for this particular instance you bring up probably won't be that hard, but the entirety of it should be replaced soon.

ericchansen commented 8 years ago

Looked at this for awhile. There's not an easy way to change the code such that this works.

  1. The class Structure has no idea what file it came from, so it can't reread the .mmo---which would be a waste of time anyway---to pick up on all the angles including those that aren't flagged with OPT.
  2. That being said, the function collect_structural_data_from_mae in calculate could be reworked to do this, but it would get messy.
  3. The entire method of picking up structural data is pretty sloppy. Honestly, the best would be to have an internal system of bonding as well as the all the XYZ coordinates and to figure it out ourselves. Then we don't need .mmo files and it would work for any software package. That being said, those changes are a little drastic and might take more time than desired.

My advice is to mark the angles with OPT but not include them in your parameters that you are optimizing.

Another option is to remove the check completely by commenting out lines 1790 - 1841 in filetypes and replace all of that with simply data.append(datum).

peonor commented 8 years ago

Another easy way out: if it didn't find the angle, the test counts as "passed". And only do the test on reference data; it's always a disaster if you should get different outcomes for FF and test. Or maybe rethink the entire concept... The major reason we have the test is to avoid mismatch, if the angle gets close to linear and MacroModel fails to output the torsion. By testing in the reference, we exclude the ones that could come close to linear. But if we're that close to start with, the torsion should have all-zero parameters in the first place... Maybe this should only be a warning, printed at the start, based on ref. data only? The thing we'd really need is on another level; for each bunch of data, whatever the type, if we have a mismatch in number of data points between reference and FF, it should always break with an informative warning. Is that test in the code right now?

peonor commented 8 years ago

Something wrong in my analysis. I made the angle OPT, still got an identical error, even though I see that the angle is now OPT in the mmo file. Something else is going on. I think I'll just comment out those lines in filetypes and see what happens.

ericchansen commented 8 years ago

The easy way out suggestion you offered is pretty simple. Adding...

ericchansen commented 8 years ago

Okay, changes made.

Do we want to leave this open until someone rewrites the torsional code? Or just close it?

peonor commented 8 years ago

Close, can always reopen if needed...