Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error in import composition #919

Closed
Pangcheng520 opened this issue Jan 28, 2024 · 6 comments
Closed

Error in import composition #919

Pangcheng520 opened this issue Jan 28, 2024 · 6 comments

Comments

@Pangcheng520
Copy link

I got an error while typing the following codes:

from matminer.featurizers.base import MultipleFeaturizer
from matminer.featurizers.import composition as cf

The error reported is: ValueError: Unexpected atomic number Z=119

Was it some kind of operator error on my part? Or is there an error in matminer?
Error in import composition.docx

@AntObi
Copy link

AntObi commented Jan 30, 2024

Hey @Pangcheng520 , this seems to be an issue caused by the latest version of Pymatgen which released last Saturday.
I've verified that pymatgen==2023.9.25 has no issues.

To resolve this issue, pip install pymatgen==2023.9.25.

@ml-evs
Copy link
Collaborator

ml-evs commented Jan 30, 2024

Yeah, unfortunately there is no-one really keeping matminer up to date with its various dependencies anymore (see also #910, #915, #918) -- I tried a bit last year but hit some roadblocks in #912.

However, the pinned dependencies in the various ./requirements/*.txt have been tested in the CI and should work for a specific version. e.g., if you are using matminer 0.9.0, you can go to the state of the repository at that tag (https://github.com/hackingmaterials/matminer/tree/v0.9.0), download the requirements file and then install matminer as pip install -r <path to relevant requirements.txt>; pip install matminer and it should use the tested versions.

Quite a few people are running into this so any volunteers to help out would be great!

@JaGeo
Copy link
Contributor

JaGeo commented Feb 6, 2024

I see. Yeah, that's a bit sad. (Automatminer is even more broken.)

Happy to help keeping it updated but, of course, help from a maintainer is needed to make this work...

@JaGeo
Copy link
Contributor

JaGeo commented Feb 6, 2024

Tagging @naik-aakash and @kaueltzen as they use matminer still quite a lot.

@ml-evs
Copy link
Collaborator

ml-evs commented Feb 6, 2024

Hi @JaGeo, I do have maintainer rights here so can merge PRs and make releases, I just don't have any real bandwidth to finish off the fixes related to #912 and the upgrade of pandas. We are still using matminer a little bit too but this is not a dealbreaker for us. I'm happy to review PRs and make releases (and can be very proactive on this) but do not want to commit my time to making fixes forever; I think perhaps this repo should be much more aggressively pinned going forwards, and then people that want to use the latest pymatgen/pandas versions with it will have to contribute it themselves.

So, for now, I think this issue is closed by your PR at #920 but this cannot come into effect until the next matminer release, which is currently blocked by pandas 2 support that breaks certain featurizers. The best approach here may just be to deprecate the featurizers that are failing and skip their tests, if no-one can fix it -- there has never been a guarantee that matminer featurizers don't change value between versions anyway but currently the failures produce all NaNs, e.g., the RDF featurizer.

@ml-evs ml-evs closed this as completed Feb 6, 2024
@ml-evs
Copy link
Collaborator

ml-evs commented Feb 6, 2024

For ongoing discussion of how to deal with this: #915 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants