ACEsuit / mace

MACE - Fast and accurate machine learning interatomic potentials with higher order equivariant message passing.
Other
413 stars 157 forks source link

Enable ASE calculator on intel. #427

Closed wcwitt closed 3 weeks ago

wcwitt commented 1 month ago

@ilyes319 @jharrymoore this is a little hacky but should enable use of the ASE calculator in Intel.

wcwitt commented 1 month ago

I don't have time to figure out the conditional import at the moment (although it shouldn't be difficult) ... would you rather merge now or wait for that?

ilyes319 commented 1 month ago

@wcwitt Thank you! Happy to merge now, but I am suprised that just this import does something. What is actually happening ?

RokasEl commented 1 month ago

I don't have time to figure out the conditional import at the moment (although it shouldn't be difficult) ... would you rather merge now or wait for that?

Just note that without a conditional import this will break MACE for users who don't have the intel_extension_for_pytorch package.

I think that at the very least the import should be surrounded by a try ... except block.

wcwitt commented 1 month ago

Just note that without a conditional import this will break MACE for users who don't have the intel_extension_for_pytorch package

Definitely true. It's possible other changes in this intel branch might have the same effect, which is why I wasn't too worried about it, but I'm not against doing things properly.

jharrymoore commented 1 month ago

Previously I think I did the import only if the device was set to xpu. Probably better is what Kelvin has done in the mace-ddp fork, something like:

from importlib.util import find_spec

has_ipex = find_spec("intel_extension_for_pytorch")
if has_ipex:
    try:
        import intel_extension_for_pytorch as ipex
    except ImportError as e:
        raise ImportError("IPEX is installed, but could not be imported!") from e
ilyes319 commented 1 month ago

happy to merge if someone does that.

wcwitt commented 1 month ago

I've just pushed something, testing now.