search

materialsvirtuallab / matgl

Graph deep learning library for materials
BSD 3-Clause "New" or "Revised" License
232 stars 57 forks source link

[Bug]: Incompataibility with `dgl==2.1.0` and `torch==2.2.2` #243

Closed Andrew-S-Rosen closed 1 month ago

Andrew-S-Rosen commented 3 months ago

Email (Optional)

No response

Version

1.0.0

Which OS(es) are you using?

What happened?

This is just a head's up that if you install matgl from PyPI right now (3/28/2024), you'll get dgl==2.1.0 and torch==2.2.2, which are not compatible. For details, see https://github.com/dmlc/dgl/issues/7247#issuecomment-2026414981.

Code snippet

No response

Log output

No response

Code of Conduct

iamusen commented 3 months ago

I've downgraded the PyTorch to ver 2.2.1, and it works.

shyuep commented 3 months ago

The pinned dependency is dgl==1.1.3. There are issues with the dgl=2.x releases.

Andrew-S-Rosen commented 3 months ago

It doesn't seem pinned:

https://github.com/materialsvirtuallab/matgl/blob/cb41e6073b56b81fd69c16a903db203d2ccaa394/pyproject.toml#L51

shyuep commented 1 month ago

The pin is in the requirements.txt file. In any case, I agree we need to update the code to support DGL 2.x. @kenko911 Pls fix.

kenko911 commented 1 month ago

Hi @shyuep and @Andrew-S-Rosen, sorry for the late reply. We need to explicitly include the dependence of pydantic starting from dgl-2. . However, the dgl-2. is not compatible with torch.2.2.2 or above at the momennt. Is it okay for us to fix it by including pydantic, torch>=2.0.0; <=2.2.1, dgl >=2.0.0 in requirement.txt?

Andrew-S-Rosen commented 1 month ago

Of course defer to @shyuep on this matter, but since users are installing matgl based on pyproject.toml and not requirements.txt, I think it is important to make sure the pyproject.toml contains the version ranges necessary to properly run matgl. The requirements.txt should have strict version pins for the sake of CI, which are updated by dependabot. In any case, I think having the suggested ranges in pyproject.toml is appropriate.

shyuep commented 1 month ago

I would go for the latest version of DGL, and whatever version the subdependencies require. My impression is that pyproject.toml. is a replacement for setup.py and you are not supposed to pin versions in pyproject.toml.

Andrew-S-Rosen commented 1 month ago

Strict version pins should be avoided in pyproject.toml, but a bounded range of versions in this case is necessary (yes?) otherwise users doing pip install matgl will not get a working build. That was the context for my original issue report, at least. I agree that the strict version pins for the latest version of DGL and necessary subdependencies should be in requirements.txt.

shyuep commented 1 month ago

That I agree. @kenko911 can modify pyproject.toml to specify the latest range.

kenko911 commented 1 month ago

Hi @shyuep and @Andrew-S-Rosen, thank you so much for your input. I will do what you all suggested in pyproject.toml with a bounded range of versions and only include the strict version in requirements.txt. I do learn a lot from you guys!! :)

Andrew-S-Rosen commented 1 month ago

Thank you for addressing this! 🙏