crash and miscalculation of score when V terms fall below 0.1

[emilywork]

Q2MM crashes when V terms fall below 0.1 during optimization. In the last complete cycle before the crash, the score is miscalculated for some trial FFs (meaning, the score calculated during the optimization differs from the score calculated by a separate compare.py with that same FF), which results in the wrong FF being selected as the "best" FF.

This is an issue I emailed about several weeks ago, and example files are included in that email.

[ericchansen]

To address this issue, please include all relevant information here.

Here are some links to read about how to submit good GitHub issues.

• https://wiredcraft.com/blog/how-we-write-our-github-issues/
• https://medium.com/nyc-planning-digital/writing-a-proper-github-issue-97427d62a20f
• https://github.com/codeforamerica/howto/blob/master/Good-GitHub-Issues.md

[jeherr]

Are these the same issues you emailed us about with the link to a zip file on the group box folder 6 days ago, or is this a different issue? I remember getting an email from you and/or @peonor maybe like three weeks ago about another bug but didn't have time to look into it then. Is this related to that email?

Also, when the code crashes, it's helpful to provide the full traceback output in a block code section like so (put ``` (backticks) on the line before and after whatever you want inside the block code)

Traceback (most recent call last):
  File "/path/to/example.py", line 4, in <module>
    greet('Chad')
  File "/path/to/example.py", line 2, in greet
    print('Hello, ' + someon)
NameError: name 'someon' is not defined

This helps us pinpoint the problem much faster since we can see what lines of code are causing the problem.

[jeherr]

Ok I'm going to try to write something up here based on the emails from @emilywork and @peonor a few weeks ago about what these bugs are and try to provide a working example to reproduce the bugs here.

It seems like the issue is somewhere along the way the parameters are getting corrupted somehow after several optimization cycles, and it likely has to do with a torsion parameter falling between -0.1 and 0.1. Emily provided an example where Q2MM would run for about 5 optimization cycles and then crash with the following traceback

Traceback (most recent call last):
  File "./loop.py", line 425, in <module>
    main(sys.argv[1:])
  File "./loop.py", line 419, in main
    loop.run_loop_input(lines)
  File "./loop.py", line 140, in run_loop_input
    self.ff = loop.opt_loop()
  File "./loop.py", line 64, in opt_loop
    self.loop_lines, score=self.ff.score)
  File "./loop.py", line 366, in run_loop_input
    self.ff = grad.run(ref_data=self.ref_data)
  File "/home/kjmf106/FF/opt.py", line 34, in wrapper
    return func(*args, **kwargs)
  File "/home/kjmf106/FF/gradient.py", line 209, in run
    r_dict, c_data = compare.trim_data(r_dict,c_data)
  File "/home/kjmf106/FF/compare.py", line 82, in trim_data
    dict1[typ].remove(datum)
AttributeError: 'numpy.ndarray' object has no attribute 'remove'

Emily then did a little debugging of this by taking the best three force fields from the final optimization cycle before it crashes and ran compare.py on all of them. The top two had different scores than what the comparison during the optimization loops, but the third best matched. Additionally, the third best has two fewer data points in the comparison data than the best two had. So somewhere some data is getting added or deleted.

Per-Ola mentioned that MacroModel used to drop torsions from the list if they went to 0, but that it no longer does. Also, apparently MacroModel used to reorder the list sometimes, and that there was a more detailed matching on the Q2MM side written to ensure that even if MacroModel reorders the list that the matching will still correctly match the reference data with the FF calculated data. I have to go to a seminar right now, so I'll come back later and try to provide a working example here to reproduce this.

[peonor]

Hi, this bug is still there in the latest version of the code. Currently, it's failing at line 94 in compare.py, and line 96 is the same. I'm not good enough at python, if I understand it right we need to remove one item in one dict in an array of dicts, and the "remove" is not a recognized command for doing that. I've tried also a solution using "del", but failing. Could someone better at python just fix these two lines?

[ericchansen]

Lines in question.

q2mm/q2mm/compare.py

Lines 88 to 98 in 7cda5c3

	for datum in to_remove:
	if datum in dict1[typ] and datum in dict2[typ]:
	raise AssertionError("The data point that is flagged to be \
	removed is present in both data sets.")
	# We may want to keep track of the data that is removed.
	if datum in dict1[typ]:
	dict1[typ].remove(datum)
	if datum in dict2[typ]:
	dict2[typ].remove(datum)
	if to_remove:
	logger.log(20, '>>> Removed Data: {}'.format(len(to_remove)))

[ericchansen]

@jeherr @jesswahlers @tquinn3 @arosale4

This code is ugly.

It looks like the code was written for us to use list.remove. See https://docs.python.org/3/tutorial/datastructures.html#more-on-lists for the docs.

So that means that dict1 and dict2 are actually lists? Or should be? That's a terrible variable name.

Assuming everything that I've said so far is true, why is our list being converted to a np.array?

A very simple and quick change might be this.

            for datum in to_remove:
                if datum in dict1[typ] and datum in dict2[typ]:
                    raise AssertionError("The data point that is flagged to be \
                    removed is present in both data sets.")
                # We may want to keep track of the data that is removed. 
                if datum in dict1[typ]:
                    dict1[typ] = list(dict1[typ]) # quick solution to make the variable type correct again
                    dict1[typ].remove(datum) 
                if datum in dict2[typ]:
                    dict2[typ] = list(dict2[typ])
                    dict2[typ].remove(datum) 
            if to_remove:
                logger.log(20, '>>> Removed Data: {}'.format(len(to_remove)))

This isn't a good, long term solution though. We should be picking our data types logically and sticking with them. They shouldn't change from a standard Python list to a np.array for funsies.

[ericchansen]

@peonor If you want to, you can make the change that I suggested in my previous comment and see if that works. I don't have the ability to run tests on Q2MM anymore since I don't have access to Schrodinger.

[jeherr]

@amille52 should be added to this. Since she will be the new Q2MM master on our side after Taylor, Kevin, myself, and the following year Jess all graduate.