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

Incompatible Pre-trained Model being pulled #80

Open
shubharajkharel opened this issue Dec 13, 2023 · 5 comments
Open

Incompatible Pre-trained Model being pulled #80

shubharajkharel opened this issue Dec 13, 2023 · 5 comments

Comments

@shubharajkharel
Copy link
Contributor

Error

Cause

  • load_model is not pulling the version of pre-trained model from matgl:v0.8.5 being used in cresendo but a newer model which includes fixes not compatible with matgl:v0.8.5

Temporary Fix

  • Replace the pre-trained model (default: M3GNet-MP-2021.2.8-PES) files cached in directory ~/.cache/matgl/ with the older pre-trained version.
@matthewcarbone
Copy link
Owner

matthewcarbone commented Dec 13, 2023

Can you post the full stack trace? This is really surprising. The default model should be version-locked so I'm not sure why this isn't working.

It also seems like this might be a bug with matgl's deployment of their model, no?

@shubharajkharel
Copy link
Contributor Author

This is really surprising.

I am too..

It also seems like this might be a bug with matgl's deployment of their model, no?

I agree, this is caused by undetected bug in matgl.

Call

featurize_material(Structure.from_file(poscar_path))

Stack Trace

Traceback (most recent call last):
  File "/Users/bnl/Downloads/dev/aimm/ai_for_xas/temp.py", line 10, in <module>
    from Crescendo.crescendo.extern.m3gnet._featurizer import featurize_material
  File "/Users/bnl/Downloads/dev/aimm/ai_for_xas/Crescendo/crescendo/extern/m3gnet/__init__.py", line 1, in <module>
    from ._featurizer import featurize_material  # noqa
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/bnl/Downloads/dev/aimm/ai_for_xas/Crescendo/crescendo/extern/m3gnet/_featurizer.py", line 58, in <module>
    def featurize_material(structure, model=_load_default_featurizer()):
                                            ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/bnl/Downloads/dev/aimm/ai_for_xas/Crescendo/crescendo/extern/m3gnet/_featurizer.py", line 53, in _load_default_featurizer
    model = load_model("M3GNet-MP-2021.2.8-PES").model
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/bnl/mambaforge/envs/aimm/lib/python3.11/site-packages/matgl/utils/io.py", line 213, in load_model
    raise ValueError(
ValueError: Bad serialized model or bad model name. It is possible that you have an older model cached. Please clear your cache by running `python -c "import matgl; matgl.clear_cache()"`

@matthewcarbone
Copy link
Owner

Have you tried clearing the cache?

@shubharajkharel
Copy link
Contributor Author

Have you tried clearing the cache?

Of course. Error message itself mentions it!

But it doesn't work because for reason I outlined above. So temporary solution involves replacing file in cache directory, which just copy pastes the working files into it.

def fix_m3gnet_version(
    model_name="M3GNet-MP-2021.2.8-PES",
    cache_dir=None,
):
    if cache_dir is None:
        cache_dir = os.path.join(os.path.expanduser("~"), ".cache", "matgl", model_name)
    valid_model_dir = os.path.join("scripts", model_name)
    if not os.path.exists(valid_model_dir):
        raise FileNotFoundError(f"Model directory {valid_model_dir} does not exist. ")
    try:
        shutil.copytree(valid_model_dir, cache_dir)
    except FileExistsError:
        warnings.warn(f"Model directory {cache_dir} already exists. ")
        warnings.warn("Overwriting existing model. ")
        shutil.rmtree(cache_dir)
        shutil.copytree(valid_model_dir, cache_dir)
    except Exception as e:
        raise e

Also note:

  • This issue is resolved in the current version. But updating it to current version creates problem in other places of current code in Cresendo starting with how Structure2Graph is implemented in newer version of matgl.
  • The fix is not about change of model, but changes in model.json and few other files.
  • Relevant changes after 0.8.5: Bug report, Pull request

@matthewcarbone
Copy link
Owner

Ok thanks for digging. Long story short, I would just use whatever featurizer that is available (probably preferable to your hack, which is nice by the way). It's not so important, in my opinion. As long as it comes from some "universal" model it will prove our point. Also that branch of Crescendo is now kinda stale and outdated. I'm not really working on it/testing it anymore.

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

2 participants