choderalab / espaloma

Extensible Surrogate Potential of Ab initio Learned and Optimized by Message-passing Algorithm 🍹https://arxiv.org/abs/2010.01196
https://docs.espaloma.org/en/latest/
MIT License
212 stars 23 forks source link

avoid pydantic 2 #170

Closed chrisiacovella closed 7 months ago

chrisiacovella commented 1 year ago

Version 2 of pydantic seems to break any code using v1. The conda-feedstock and dev environment yaml files should be update to ensure using <2.

mikemhenry commented 1 year ago

We should be okay since we don't actually have pydantic as a dependency.

$ grep -r "pydantic"
scripts/perses-benchmark/espaloma-perses.export.yaml:  - pydantic=1.9.0=py39hb9d737c_1

The only hit looks like an export of a yaml that would include the entire environment.

chrisiacovella commented 1 year ago

I think this issue might come from packages espaloma depends on that don't adequately pin versions. A quick mamba install espaloma has it trying to install 2.0.2:

+ pydantic 2.0.2 py310h2aa6e3c_1 conda-forge 517kB

chrisiacovella commented 1 year ago

Which leads to issues when trying to import the espaloma...looks to be from qcelemental

mikemhenry commented 1 year ago

Okay I think if packages we depend on by the time we cut the next conda-forge release don't have the pin, we should pin it but in a perfect world they should add a pin.

@chrisiacovella can you raise an issue on the feedstock and main repo to ask them to add the pin? They might need to patch the repo metadata.

The main reason I want to avoid doing this if we can (but I do understand we need to do it if our dependencies don't) is that when packages to fix their support for pydantic 2.x, we will either be pulling in older packages or not able to create an env if all the deps have moved to pydantic 2.x and we still have the pin, which then requires us to remove it.

mikemhenry commented 1 year ago

Resolved in #172

chrisiacovella commented 1 year ago

It looks like they updated it 2 weeks ago to require less <2. So the behavior is strange.

If I conda install qcelemental, it will grab the most current version (qcelemental-0.25.1-pyhd8ed1ab_2) with the pinned pydantic version.

If I conda install espaloma or conda install qcportal(as qcelemental is a dependency) it will grab the 8 month old version (qcelemental-0.25.1-pyhd8ed1ab_1) and pydantic 2.0.2

So it looks like the issue is that qcportal doesn't have a max version of pydantic. I'm still very new to dealing with the Conda-forge feedstocks, so is behavior (ignoring dependency limits) pretty, uh, typical?

If I added qcelemental before qcportal in the espaloma environment file, I get the correct behavior. So is this basically just an order operations issue? the qcportal feedstock has pydantic first...so it install pydantic 2, and realizes it can resolve the conflict in qcelemental by grabbing the older version?

- pydantic >=1.4
- qcelemental >=0.13.1
chrisiacovella commented 1 year ago

I just submitted an issue to qcportal, so hopefully they will resolve (either pinning or changing the order of dependencies in the meta.yaml). Thanks for your input @mikemhenry . I'm guessing it makes sense leaving this issue open until qcportal fixes things, even though it has been resolved (i.e., not actually espaloma's issue).

mikemhenry commented 1 year ago

Keeping it open is nice as well since if someone searches for this issue they might find it and see we are working on it.

mikemhenry commented 1 year ago

Can you link the qcportal issue here?

chrisiacovella commented 1 year ago

Link to relevant qcportal issue:

https://github.com/conda-forge/qcportal-feedstock/issues/31

mikemhenry commented 1 year ago

Good call btw in getting that pin into our own feedstock since upstream hasn't fixed it yet :+1:

mattwthompson commented 1 year ago

Recommend anywhere in your source code using

try:
    from pydantic.v1 import BaseModel
except ImportError:
    from pydantic import BaseModel

as this allows the same code to work with both installed versions. OpenFF is rolling out this change after seeing Levi and Lori do it in the QCArchive stack. Downsides are

https://docs.pydantic.dev/2.0/migration/

mikemhenry commented 1 year ago

doesn't fix the problem if it's actually in upstreams

This is really our current issue since we don't use pydantic directly (at least in this repo).

Thanks!

ijpulidos commented 7 months ago

As @mikemhenry said, we are not really using pydantic for this code base. Closing. Feel free to reopen if something else comes up related to this. Thanks!