Skip to content

MACE interface, vol. 2 - #242

Merged
danielhollas merged 73 commits into
masterfrom
dh-mace
Aug 9, 2026
Merged

MACE interface, vol. 2#242
danielhollas merged 73 commits into
masterfrom
dh-mace

Conversation

@danielhollas

@danielhollas danielhollas commented May 30, 2026

Copy link
Copy Markdown
Contributor

This is a continuation from #224, with the following changes

  • remove &mace section from the abin input file, instead the path to the model and other configuration is passed via command line parameter to mace_server.py. This is specified at the top of utils/run.mace_abin.sh
  • Fix coordinates received from ABIN (without this the interface was simply not working)
  • Use MACECalculator instead of calling pytorch directly
  • Fix float32 evaluation
  • removed install_mace.sh and the MACE-OFF model file
  • test mace_server.py in MACE test directly
  • lot's of error-handling improvements
  • add tests for error paths

TODO:

  • Test run.mace_abin.sh
  • Let Veronika test everything
  • Check size of incoming coordinates from ABIN in mace_server.py
  • Use logging module?

@danielhollas
danielhollas requested review from FelyCZ and JanosJiri July 8, 2026 18:42
@danielhollas

Copy link
Copy Markdown
Contributor Author

Okay, I think this is mostly ready now for merging! We'll do some internal testing. @FelyCZ would be great if you could have a look. I summarized the changes I made in the PR description.

@JanosJiri up to you how much you want to review this. Perhaps at least the README files and mace_server.py for clarity. I'll ask people here for review as well.

@JanosJiri JanosJiri left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @danielhollas, I'm sorry but I struggle with time now and I can't make a useful review in a short time. I checked README.md though and it looks clean and understandable to me. Sorry.

@FelyCZ FelyCZ mentioned this pull request Aug 5, 2026
@FelyCZ

FelyCZ commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

Hello, thanks for revising the MACE interface!
I think it is now ready to be released. I have left you some comments and implemented the logging module (see PR #257).


energy, forces = mace_model.evaluate(atom_types, coords)

end = perf_counter()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to include torch.cuda.synchnronize() when the computation is running on CUDA. Otherwise is won't be measuring the time correctly.

if config.device == "cuda":
    import torch
    torch.cuda.synchronize() 

end = perf_counter()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it really needed? Surely mace_model.evaluate cannot return the final result until CUDA kernels finished?

Comment thread src/force_mace.F90
! ----------------------------------------------------------------
use mod_const, only: DP
use mod_error, only: fatal_error
use mod_files, only: stdout, stderr

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move natqm to global scope (as it is used in both subroutines)?

use mod_qmmm, only: natqm

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we could, but it also is nice to have it imported locally as it's closer to its use. I'll leave it like that for now.

@danielhollas

Copy link
Copy Markdown
Contributor Author

@FelyCZ thanks for taking a look! Let's merge this, and we can continue with tweaks in #257.

We've tested this locally with our own MACE model. We might need to do further testing with various foundational model but that can be a follow-up as well.

Thanks for your original work on this @FelyCZ! I think this is going to be super-useful going forward.

@danielhollas
danielhollas merged commit 4fd3c87 into master Aug 9, 2026
17 checks passed
@danielhollas
danielhollas deleted the dh-mace branch August 9, 2026 16:13
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.

3 participants