CederGroupHub / chgnet

Pretrained universal neural network potential for charge-informed atomistic modeling https://chgnet.lbl.gov
https://doi.org/10.1038/s42256-023-00716-3
Other
232 stars 62 forks source link

[Bug]: pypi release 0.2.0 doesn't match GitHub release #125

Closed mwolinska closed 7 months ago

mwolinska commented 7 months ago

Version

v0.2.0

Which OS(es) are you using?

What happened?

  1. I have previously used version 0.2.0 for my project, but when recreating an environment and installing the version CrystalGraphConverter.forward() method no longer took the "on_isolated_atoms" argument. I saw that this was an update in version 0.2.1.

I checked the pypi distribution files and the chgnet-0.2.0-cp39-cp39-macosx_10_9_x86_64.whl and chgnet-0.2.0-cp39-cp39-manylinux_2_17_x86_64.manylinux2014_x86_64.whl versions contained code consistent with the 0.2.1 release.

I noticed that the pypi distribution files were updated the day before the 0.2.1 release, so I suspect the 0.2.0 release was overwritten.

image
  1. Proposed resolution: republish version 0.2.0

Code snippet

No response

Log output

No response

Code of Conduct

BowenD-UCB commented 7 months ago

If you want to keep using CHGNet v0.2.0 version, you can install the latest release by pip install chgnet, And access the 0.2.0 version model by chgnet = CHGNet.load('0.2.0') The latest release should support the on_isolated_atoms key

There have been many small improvements and bug fixes since 0.2.0 release, I would recommend using the most recent release even if you want to keep using 0.2.0 version model weights. Is there a specific reason that 0.2.0 pypi release is needed?

mwolinska commented 7 months ago

Hi Bowen, Thanks for your help with this. The main concern is that it seems like publishing the wheels for v0.2.1 also overwrote the wheels for 0.2.0 in the process. In the image I posted you can clearly see that the wheels pypi has for 0.2.0 were published at 2 different dates. To double check that that was the case, I've also downloaded and inspected the wheels (from pypi) and the code in there doesn't align with the v0.2.0 release on your GitHub.

The point for this issue is twofold:

In any case, I'll be moving on to the latest version as it does seem to work very nicely on my side ! :) Hope that helps!

BowenD-UCB commented 7 months ago

Thanks for clarify that. Since we have good backward compatibility, I think we don't need to fix this at the moment. @janosh Can you please also take a look if the 0.2.0 release needs to be fixed?

janosh commented 7 months ago

i don't remember the details but we definitely had some hiccups in the v0.2.0 PyPI release when transitioning to the fast cythonized graph constructor which required making platform specific wheels for mac, win, linux instead of a simple source distribution.

that said, you can't overwrite a previous pypi release. at most we could make a new release with the same version but higher build number (see build tag docs) which is very rarely used but might fit our use case here. i'm leaning more towards just yanking the 0.2.0 release to discourage people from using it since it contains known bugs (#79).

BowenD-UCB commented 7 months ago

I agree with yanking the 0.2.0 version

janosh commented 7 months ago

@BowenD-UCB yanked v0.2.0.