selimsami / qforce

Apache License 2.0
57 stars 13 forks source link

Python ValueError when running propane example #67

Closed mattiafelice-palermo closed 6 months ago

mattiafelice-palermo commented 6 months ago

Dear Selim,

first of all thank you very much for sharing qforce with the opensource community.

I'm trying to reproduce the propane example before embarking on more complex molecules, but I cannot move past the QM dihedral calculations.

After performing, the dihedral QM PES scan, I run qforce propane, but unfortunately the program halts with the following traceback:

Calculating the MD hessian matrix elements...
Fitting the MD hessian parameters to QM hessian values
Done!

Traceback (most recent call last):
  File "/home/user/anaconda3/envs/qforce-pip/bin/qforce", line 8, in <module>
    sys.exit(run())
 [...]
  File "/home/user/anaconda3/envs/qforce-pip/lib/python3.9/site-packages/qforce/fragment.py", line 341, in make_fragment_terms
    self.non_bonded = mol.non_bonded.subset(mol.non_bonded, self.frag_charges, map_mol_to_db)
  File "/home/user/anaconda3/envs/qforce-pip/lib/python3.9/site-packages/qforce/molecule/non_bonded.py", line 91, in subset
    if frag_charges != []:
ValueError: operands could not be broadcast together with shapes (11,) (0,)

I did some debugging, and found the issue to be in the subset class method from the molecule/non_bonded.py module. At line 90, there's an if clause to see if fragment charges has value (if frag_charges != []:), but it seems the code attempts to compare a numpy array with a python list, resulting in a ValueError.

I modified the code converting on the fly the frag_charges numpy array into a list just for the if clause, and with this modification qforce runs as expected and finishes the job gracefully.

It seems strange that this kind of bug is out in the open, perhaps there has been recent numpy library changed that became more restrictive and prevent the type mixing as shown above? Or maybe I'm just doing something wrong...

Thank you and I hope this is useful.

Mattia

selimsami commented 6 months ago

Hi Mattia,

Thanks a lot for bringing this up. As you suggested it must have been a recent change in one of the dependencies - sign that I should be adding more tests! I have now updated the code to fix this issue. Hopefully the rest of your experience with qforce will be smoother, feel free to ask if you have any issues/questions!

Cheers, Selim

mattiafelice-palermo commented 6 months ago

Perfect thank you very much for your help, it's much appreciated!

Mattia