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

repair CI #463

Merged
merged 5 commits into from
Jan 14, 2025
Merged

repair CI #463

merged 5 commits into from
Jan 14, 2025

Conversation

loriab
Copy link
Collaborator

@loriab loriab commented Jan 14, 2025

Description

Fix CI by updating conda provider. Also, the most recent MACE breaks the harness, so downgrade it for now.

Changelog description

Status

  • Code base linted
  • Ready to go

Copy link

codecov bot commented Jan 14, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.48%. Comparing base (76e8b2a) to head (d2a0ec7).

Additional details and impacted files

@loriab loriab marked this pull request as ready for review January 14, 2025 07:44
@loriab loriab changed the title test CI repair CI Jan 14, 2025
@loriab
Copy link
Collaborator Author

loriab commented Jan 14, 2025

@jthorton

FYI, pymace has been upgrading, and current qcng harness gets the error below involving pytorch. I've pinned the CI to an older patch version for now.

        result = qcng.compute(atomic_input, "mace")
>       assert result.success
E       assert False
E        +  where False = FailedOperation(error=ComputeError(error_type='unknown_error', error_message='QCEngine Execution Error:\nTraceback (most recent call last):\n  File "/home/runner/work/QCEngine/QCEngine/qcengine/util.py", line 117, in compute_wrapper\n    yield metadata\n  File "/home/runner/work/QCEngine/QCEngine/qcengine/compute.py", line 108, in compute\n    output_data = executor.compute(input_data, config)\n  File "/home/runner/work/QCEngine/QCEngine/qcengine/programs/mace.py", line 95, in compute\n    model, r_max, atomic_numbers = self.load_model(name=method)\n  File "/home/runner/work/QCEngine/QCEngine/qcengine/programs/mace.py", line 70, in load_model\n    comp_mod = jit.compile(model)\n  File "/usr/share/miniconda/envs/test/lib/python3.10/site-packages/e3nn/util/jit.py", line 99, in compile\n    compile(\n  File "/usr/share/miniconda/envs/test/lib/python3.10/site-packages/e3nn/util/jit.py", line 99, in compile\n    compile(\n  File "/usr/share/miniconda/envs/test/lib/python3.10/site-packages/e3nn/util/jit.py", line 99, in compile\n    compile(\n  File "/usr/share/miniconda/envs/test/lib/python3.10/site-packages/e3nn/util/jit.py", line 111, in compile\n    mod = torch.jit.script(mod, **script_options)\n  File "/usr/share/miniconda/envs/test/lib/python3.10/site-packages/torch/jit/_script.py", line 1429, in script\n    ret = _script_impl(\n  File "/usr/share/miniconda/envs/test/lib/python3.10/site-packages/torch/jit/_script.py", line 1147, in _script_impl\n    return torch.jit._recursive.create_script_module(\n  File "/usr/share/miniconda/envs/test/lib/python3.10/site-packages/torch/jit/_recursive.py", line 557, in create_script_module\n    return create_script_module_impl(nn_module, concrete_type, stubs_fn)\n  File "/usr/share/miniconda/envs/test/lib/python3.10/site-packages/torch/jit/_recursive.py", line 634, in create_script_module_impl\n    create_methods_and_properties_from_stubs(\n  File "/usr/share/miniconda/envs/test/lib/python3.10/site-packages/torch/jit/_recursive.py", line 466, in create_methods_and_properties_from_stubs\n    concrete_type._create_methods_and_properties(\nRuntimeError: Can\'t redefine method: forward on class: __torch__.mace.modules.irreps_tools.reshape_irreps (of Python compilation unit at: 0x56072d57d980)\n')).success

@jthorton
Copy link
Contributor

Thanks for the ping @loriab, see the related issue here. I can patch the interface to use the method described if you like? I was planning on adding the ability to calculate the hessian with MACE as well anyway.

@loriab
Copy link
Collaborator Author

loriab commented Jan 14, 2025

Thanks for the ping @loriab, see the related issue ACEsuit/mace#741. I can patch the interface to use the method described if you like? I was planning on adding the ability to calculate the hessian with MACE as well anyway.

Ah, thanks for the linked issue @jthorton . Glad it's not solely a harness error. I'm fine with the pin if you want to wait on the hessian + MACE + any other fixes. I have some ambitions of getting a small release out this week.

@jthorton
Copy link
Contributor

I don't want to slow down the release process, so going ahead with the pin for now would be best! I'll make an issue to remind me to update the harness.

@loriab loriab merged commit b56a2aa into master Jan 14, 2025
19 checks passed
@loriab loriab deleted the loriab-patch-2 branch January 14, 2025 18:38
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

Successfully merging this pull request may close these issues.

2 participants