search

xiaohang007 / SLICES

SLICES: An Invertible, Invariant, and String-based Crystal Representation [2023, Nature Communications]
https://www.nature.com/articles/s41467-023-42870-7
GNU Lesser General Public License v2.1
49 stars 7 forks source link

Switch from `m3gnet` to `matgl`. #9

Open hongyi-zhao opened 2 weeks ago

hongyi-zhao commented 2 weeks ago

M3gnet is used by this package currently, but as noted here:

NOTE: A new implementation based on the Deep Graph Library and PyTorch called the Materials Graph Library (MatGL) has replaced this implementation. This repository has been archived and will no longer be maintained. It will be kept purely as a reference implementation. Users are recommended to use matgl instead.

So, it seems that we should use matgl instead.

Regards, Zhao

xiaohang007 commented 2 weeks ago

m3gnet is used by this package currently, but as noted here:

NOTE: A new implementation based on the Deep Graph Library and PyTorch called the Materials Graph Library (MatGL) has replaced this implementation. This repository has been archived and will no longer be maintained. It will be kept purely as a reference implementation. Users are recommended to use matgl instead.

So, it seems that we should use matgl instead.

Regards, Zhao

Thank you for your suggestion. I have conducted an internal test on SLICES with matgl, and it turned out to be around 10x slower than the CHGNet implementation in https://github.com/CederGroupHub/chgnet/tree/main/chgnet when running the benchmark of MP20, which is pretty weird.

xiaohang007 commented 2 weeks ago

m3gnet is used by this package currently, but as noted here:

NOTE: A new implementation based on the Deep Graph Library and PyTorch called the Materials Graph Library (MatGL) has replaced this implementation. This repository has been archived and will no longer be maintained. It will be kept purely as a reference implementation. Users are recommended to use matgl instead.

So, it seems that we should use matgl instead.

Regards, Zhao

Do you think it is possible to integrate SLICES encoding and decoding in pymatgen, similar to the SMILES encoding and decoding functions in RDKIT?

hongyi-zhao commented 2 weeks ago

Thank you for your suggestion. I have conducted an internal test on SLICES with matgl, and it turned out to be around 10x slower than the CHGNet implementation in https://github.com/CederGroupHub/chgnet/tree/main/chgnet when running the benchmark of MP20, which is pretty weird.

Matgl is still under active development. Perhaps you should commit a issue to discuss this problem with the developers in-depth to identify the bottleneck.

Do you think it is possible to integrate SLICES encoding and decoding in pymatgen, similar to the SMILES encoding and decoding functions in RDKIT?

To be frank, I've no idea. But I also think that this is a wonderful idea and should be discussed with the pymatgen's core developer team.

xiaohang007 commented 2 weeks ago

Thank you for your suggestion. I have conducted an internal test on SLICES with matgl, and it turned out to be around 10x slower than the CHGNet implementation in https://github.com/CederGroupHub/chgnet/tree/main/chgnet when running the benchmark of MP20, which is pretty weird.

Matgl is still under active development. Perhaps you should commit a issue to discuss this problem with the developers in-depth to identify the bottleneck.

Do you think it is possible to integrate SLICES encoding and decoding in pymatgen, similar to the SMILES encoding and decoding functions in RDKIT?

To be frank, I've no idea. But I also think that this is a wonderful idea and should be discussed with the pymatgen's core developer team.

Thanks a lot. I will commit a issue to discuss this problem with the developers.