Upgrade the aimsgb structure factory #1123
Conversation
| @@ -2,8 +2,10 @@ | |||
| # Copyright (c) Max-Planck-Institut für Eisenforschung GmbH - Computational Materials Design (CM) Department | |||
| # Distributed under the terms of "New BSD License", see the LICENSE file. | |||
|
|
|||
| from structuretoolkit.build import grainboundary, get_grainboundary_info | |||
There was a problem hiding this comment.
Please use the structure toolkit interface.
There was a problem hiding this comment.
The interface is pretty similar and this is what is used by pyiron_atomistics in the background https://github.com/pyiron/structuretoolkit/blob/main/structuretoolkit/build/aimsgb.py
There was a problem hiding this comment.
I opened a jupyter notebook from the jupyterhub session runnig at cmti and tried to import from structuretoolkit.common.pymatgen import ase_to_pymatgen, pymatgen_to_ase and got this error ModuleNotFoundError: No module named 'structuretoolkit.common'
from structuretoolkit.build import grainboundary, get_grainboundary_info also gave me the error ImportError: cannot import name 'grainboundary' from 'structuretoolkit.build' (/u/system/SLES12/soft/pyiron/dev/anaconda3/lib/python3.10/site-packages/structuretoolkit/build/__init__.py)
There was a problem hiding this comment.
Oh, I got this. I have to modify the import statement from from structuretoolkit.build import grainboundary, get_grainboundary_info to from structuretoolkit.build.aimsgb import grainboundary_info, grainboundary_build
But, still can't figure out what is the new alternative of structuretoolkit.common.pymatgen import ase_to_pymatgen, pymatgen_to_ase
There was a problem hiding this comment.
Now I am confused, because it is available in the official package https://github.com/pyiron/structuretoolkit/blob/main/structuretoolkit/common/pymatgen.py . Which version of structure toolkit is installed on cmti? You can check the version using conda list structuretoolkit
There was a problem hiding this comment.
The output of conda list structuretoolkit is as follows:
packages in environment at /u/system/SLES12/soft/pyiron/dev/anaconda3:
Name Version Build Channel
structuretoolkit 0.0.1 pyhd8ed1ab_1 conda-forge
Note: you may need to restart the kernel to use updated packages.
There was a problem hiding this comment.
Ok, this is a rather old version - the current version is 0.0.7.
There was a problem hiding this comment.
@jan-janssen Thanks! I noticed that the current incompatibility issue is coming from this line AseAtomsAdaptor.get_structure(initial_struct, cls=None). The option cls=None is no longer accepted. We can change it to AseAtomsAdaptor.get_structure(initial_struct) which will take the default value which is pymatgen.Structure
|
Are you sure that it’s related to numpy? Because recently Aditya reported a problem with aimsgb and I fixed it here. I don’t know the current status, but if the new version is released the problem might get solved as well? |
|
I believe the current error is coming from |
There was a problem hiding this comment.
Need black applied, and I see in the review comments that there are some other little issues going on, but overall it looks fine to me.
At least for info, where we're really just wrapping things, I wonder whether we can get away with the following such that the method always stays up to date with changes in the underlying wrapped function:
class AimsgbFactory:
@staticmethod
@functools.wraps(get_grain_boundary_info)
def info(*args, table_view=False, **kwargs):
if table_view:
return print(get_grain_boundary_info(*args, **kwargs).__str__())
else:
return get_grain_boundary_info(*args, **kwargs)This works like a charm when I make a little MWE in my notebook, but try it and see if it's good here.
| """ | ||
| return get_grainboundary_info(axis=axis, max_sigma=max_sigma) | ||
| if table_view: | ||
| return print(GBInformation(axis, max_sigma, specific).__str__()) |
There was a problem hiding this comment.
return print(...) looks super weird. Is it really not enough to just return __str__()? or print() without the return?
There was a problem hiding this comment.
@liamhuber Thanks a lot for the feedback. I will implement the changes you suggested.
|
I tried to upgrade to $ mamba install sturcturetoolkit=0.0.6
...
- pyiron_atomistics 0.2.66 pyhd8ed1ab_1 conda-forge
+ pyiron_atomistics 0.2.64 pyhd8ed1ab_0 conda-forge/noarch 260kB
- pyiron_contrib 0.1.10 pyhd8ed1ab_0 conda-forge
+ pyiron_contrib 0.1.8 pyhd8ed1ab_2 conda-forge/noarch 167kBwhich made $ conda list | grep pyiron
# packages in environment at /u/system/SLES12/soft/pyiron/dev/anaconda3/:
pyiron 0.4.7 pyhd8ed1ab_1 conda-forge
pyiron-data 0.0.22 hd8ed1ab_0 conda-forge
pyiron_atomistics 0.2.66 pyhd8ed1ab_1 conda-forge
pyiron_base 0.5.36 pyhd8ed1ab_0 conda-forge
pyiron_contrib 0.1.10 pyhd8ed1ab_0 conda-forgewhich revealed, that actually none of the pyiron packages is really up to date, so I tried to upgrade everything explicitly: $ mamba install structuretoolkit=0.0.6 pyiron=0.5.0 pyiron-data=0.0.22 pyiron_atomistics=0.3.1 pyiron_base=0.6.3 pyiron_contrib=0.1.10
...
Encountered problems while solving:
- package pyiron_base-0.6.3-pyhd8ed1ab_0 requires pysqa >=0.1.0, but none of the providers can be installedSo, why can't we just upgrade $ mamba install structuretoolkit=0.0.6 pyiron=0.5.0 pyiron-data=0.0.22 pyiron_atomistics=0.3.1 pyiron_base=0.6.3 pyiron_contrib=0.1.10 pysqa=0.1.0
...
Encountered problems while solving:
- package pysqa-0.1.0-pyhd8ed1ab_0 requires paramiko >=3.2.0, but none of the providers can be installedAll right, here we go again... $ mamba install structuretoolkit=0.0.6 pyiron=0.5.0 pyiron-data=0.0.22 pyiron_atomistics=0.3.1 pyiron_base=0.6.3 pyiron_contrib=0.1.10 pysqa=0.1.0 'paramiko>=3.2.0'
...
Encountered problems while solving:
- package pysqa-0.1.0-pyhd8ed1ab_0 requires paramiko >=3.2.0, but none of the providers can be installedThat didn't work as intended. So I tried just installing $ mamba install "paramiko>=3.2.0"
Encountered problems while solving:
- package fabric3-1.14.post1-py_0 requires paramiko >=2.0,<3.0, but none of the providers can be installed
As far as I can tell, it's just a legacy package, I did not find anything depending on it: $ conda-tree whoneeds fabric3returns nothing. Shall I just uninstall it? |
|
I uninstalled it. Now, all pyiron packages as well as structuretoolkit (and also some other crucial packages like pysqa) are up to date. |
Pull Request Test Coverage Report for Build 5776408730
💛 - Coveralls |
|
@jan-janssen and @liamhuber thanks a lot for your suggestions! I have implemented them. I also noticed that some core methods of |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
The current version of
amisgb.buildandaimsgb.infois not compatible with probablyPython3.10. Here, I upgrade the aimsgb structure factory to make it compatible and also add some more features currently available inAimsgb(e.g., to add vacuum spacing in the GB cell).