Rebuild coordinate system may fail for PrimitiveInternalCoordinates

[jsjyhzy]

When optimizing with coordsys="prim", it will occasionally raise the error 'PrimitiveInternalCoordinates' object has no attribute 'conmethod' from the following line

geomeTRIC/geometric/optimize.py

Line 219 in 9981b39

IC1 = self.IC.__class__(newmol, connect=self.IC.connect, addcart=self.IC.addcart, build=False, conmethod=self.IC.conmethod)

And it seems, in prim, the conmethod was never be used

geomeTRIC/geometric/internal.py

Lines 2005 to 2024 in 9981b39

	class PrimitiveInternalCoordinates(InternalCoordinates):
	def __init__(self, molecule, connect=False, addcart=False, constraints=None, cvals=None, **kwargs):
	super(PrimitiveInternalCoordinates, self).__init__()
	self.connect = connect
	self.addcart = addcart
	self.Internals = []
	self.cPrims = []
	self.cVals = []
	self.Rotators = OrderedDict()
	self.elem = molecule.elem
	# List of fragments as determined by residue ID, distance criteria or bond order
	self.frags = []
	for i in range(len(molecule)):
	self.makePrimitives(molecule[i], connect, addcart)
	# Assume we're using the first image for constraints
	self.makeConstraints(molecule[0], constraints, cvals)
	# Reorder primitives for checking with cc's code in TC.
	# Note that reorderPrimitives() _must_ be updated with each new InternalCoordinate class written.
	self.reorderPrimitives()

in which case the creation of PrimitiveInternalCoordinates at first time would be fine, while re-creation of it fails.

I wonder if it is the problem in optmize.py which could be fixed easily, or the so called conmethod is simply not implemented for PrimitiveInternalCoordinates yet?

[leeping]

Thanks for the issue report, you are correct that this line contains a mistake:

geomeTRIC/geometric/optimize.py

Line 219 in 9981b39

IC1 = self.IC.__class__(newmol, connect=self.IC.connect, addcart=self.IC.addcart, build=False, conmethod=self.IC.conmethod)

The context is that

1. coordsys="prim" is mainly implemented for reference in geomeTRIC and is not as high-performing as other options such as coordsys="tric" or coordsys="dlc". Therefore it is recommended that you use the default coordinate system or one of the two latter options.

2. The conmethod keyword refers to the algorithm for constrained optimization, and constrained optimizations are only possible when using coordsys="tric" or coordsys="dlc". The conmethod attribute does not exist in the other coordinate systems. Therefore, line 219 of optimize.py is a bug that causes errors only when using coordinate systems other than "tric" or "dlc". I missed this bug because I seldom use any coordinate system other than those two.

To fix the bug, you should be able to change the code to something like this:

IC1 = self.IC.__class__(newmol, connect=self.IC.connect, addcart=self.IC.addcart, build=False, conmethod=self.IC.conmethod if hasattr(self.IC, 'conmethod') else 0) 

It will first check to see if the conmethod attribute exists before using its value. If the attribute does not exist then it will use zero and the PrimitiveInternalCoordinates class should ignore it. You can submit a PR with this fix or I can fix it myself when I have a free moment.

Thanks,

• Lee-Ping

[jsjyhzy]

Thank you for your comprehensive explanation, the PR #161 is submitted, and I thought this would be a little bit more elegant.

[leeping]

Thank you. :) I merged your changes. Thanks for your patience.