search

peteboyd / lammps_interface

automatic generation of LAMMPS input files for molecular dynamics simulations of MOFs
MIT License
125 stars 63 forks source link

cut_molecule #40

Closed michelleernst closed 3 years ago

michelleernst commented 3 years ago

Dear all, I was trying to use the lammps interface with the test cif IRMOF-1.cif you provide in the test folder (as well as with some other cifs). For all the tested cif files I get this error message: _main.py", line 1532, in cut_molecule mgraph.coordinates = self.graph.coordinates[indices,:].copy() TypeError: 'NoneType' object is not subscriptable I would be very glad for help to fix this issue. Kind regards, Michelle Ernst

ltalirz commented 3 years ago

Thanks for the report @michelleernst - this really highlights the fact that we absolutely should be adding automatic tests, which would prevent such failures on the test structures in the repository.

I've just added a pull request that starts running some of those and I'm able to reproduce your issue. Interestingly, it seems to depend on the python version - in a fresh local python 3.6 environment, the code runs fine, while in a python 3.8 environment it doesn't.

Edit: The only diff in the dependencies that I can find: python3.6 includes two extra dependencies, namely importlib-metadata==1.7.0 and zipp==3.1.0. This should be system-level stuff and not affect this isue.

The error comes from the self.graph object which is defined as a subclass of networkx.Graph, i.e. I assume the python-version-dependent behavior happens inside networkx.

ltalirz commented 3 years ago

@peteboyd It seems to me there is some remaining issue with the upgrade to networkx 2.4

Would you mind having a look into this? Essentially, just create a python 3.8 environment and adapt lammps_interface to what networkx expects (it looks like, currently, lammps_interface is broken on python 3.8).

@michelleernst In the meanwhile, you should be able to use the code in a python 3.6 environment (which you can create, for example, using conda)

peteboyd commented 3 years ago

It was an issue with the module time. A while ago had merged a pull request that printed the time for computing each force field element in a cif file. It was calling time.clock() which is apparently deprecated.

ltalirz commented 3 years ago

Thanks @peteboyd for fixing the issue! @michelleernst As demonstrated by the passing tests in https://github.com/peteboyd/lammps_interface/pull/41 , the current master branch of the repository should work. Once the PR is merged, I will make a 1.4.0 release on PyPI as well.

ltalirz commented 3 years ago

I've just pushed out v0.2.0 to pypi. Should be available in a few minutes.