Skip to content

Add PolarMACE support to MaceTorchSimModel#1441

Open
orionarcher wants to merge 1 commit into
ACEsuit:developfrom
orionarcher:orionarcher/torchsim-polarizability
Open

Add PolarMACE support to MaceTorchSimModel#1441
orionarcher wants to merge 1 commit into
ACEsuit:developfrom
orionarcher:orionarcher/torchsim-polarizability

Conversation

@orionarcher

Copy link
Copy Markdown

Summary

  • Wire PolarMACE-specific inputs (rcell, volume, external_field, fermi_level, density_coefficients) through the TorchSim interface with sensible defaults matching AtomicData.from_config()
  • Auto-detect PolarMACE models via isinstance check (matching the ASE calculator pattern) and forward additional outputs (charges, dipole, density_coefficients, etc.) in the results dict
  • Include padding support for polar tensors in the torch.compile path

Design decisions

  • Auto-detection via isinstance(model, PolarMACE) — zero user burden, matches ASE calculator pattern
  • Defaults match MACE's own AtomicData.from_config(): zeros for external_field, 0.0 for fermi_level, zeros(n_atoms, 1) for density_coefficients
  • external_E_field read from TorchSim SimState extras (via SystemExtras.EXTERNAL_E_FIELD), mapped to external_field in MACE's data_dict
  • Cell format unchanged — both MACE and PolarMACE use prepare_graph() with cell.view(-1, 3, 3), so existing [n_systems, 3, 3] format works for both

Test plan

  • test_torchsim_polar_basic — forward pass with defaults (no extras)
  • test_torchsim_polar_with_extras — forward pass with explicit extras
  • test_torchsim_polar_no_extras_vs_zero_extras — verify defaults match zero extras
  • test_torchsim_polar_batched — batched multi-system forward pass
  • test_torchsim_polar_batched_with_extras — batched with per-system extras
  • test_torchsim_polar_matches_ase — compare against ASE MACECalculator(model_type="PolarMACE")
  • Existing test_torchsim.py tests should pass unchanged (CI verification needed)

@orionarcher

Copy link
Copy Markdown
Author

@janosh would you mind taking a look at this? Trying to get MACE Polar working here.

@janosh

janosh commented Apr 14, 2026

Copy link
Copy Markdown
Contributor

Thanks for working on this @orionarcher!

  1. polarizability and polarizability_sh are missing from per_system_keys. i think in the torch.compile padded path they'll be returned un-sliced, including padding garbage?

  2. _build_polar_data calls torch.linalg.inv unconditionally on the cell, but AtomicData.from_config() guards against zero-volume cells (non-periodic systems) by falling back to zeros(3, 3). could error or produce NaN for molecules

  3. density_coefficients defaults to zeros(n_atoms, 1), hardcoding the second dimension to 1. But the actual dimension depends on the model's atomic_multipoles_max_l — e.g. if max_l=1, the dimension is (1+1)^2 = 4, not 1. The ASE calculator reads this from the model (self.density_dim = (model.atomic_multipoles_max_l + 1) ** 2). i think we need do the same here to avoid a shape mismatch when a model with max_l > 0 is used.

  4. isinstance(model, PolarMACE) for auto-detection is probably fine for now but could break later this year. there are plans to consolidate MACE model classes. maybe better to detect capability via a model attribute (e.g. hasattr(model, 'use_polarizability') or similar flag) rather than relying on the class hierarchy. @ilyes319 would know best

  5. AtomicData.from_config() defaults total_spin to 1.0 while SimState.spin defaults to 0.0. your ASE comparison test handles this correctly, but users won't know to align them. probably worth a log warning when _is_polar and spin == 0.

  6. np.random.RandomState(0) in tests is deprecated in favor of np.random.default_rng(seed=0).

@orionarcher
orionarcher force-pushed the orionarcher/torchsim-polarizability branch from 7ceb41c to ee0e3a7 Compare April 16, 2026 23:20
ilyes319 added a commit that referenced this pull request May 3, 2026
Cherry-picks the polar-model functionality from #1441 without the
unrelated linter/style sweep:

- Auto-detect PolarMACE via isinstance and forward PolarMACE-specific
  inputs (rcell, volume, external_field, fermi_level, density_coefficients)
  with sensible defaults that match AtomicData.from_config().
- Read external_E_field from SimState extras when present.
- Expose PolarMACE outputs (charges, dipole, density_coefficients,
  polarizability, etc.) in the results dict.
- Pad the new tensors inline in _fill_padded_data, matching the
  existing displacement pattern, so the torch.compile path works for
  polar models without changing the function signature or introducing
  new persistent buffers.

Tests: add the 6 polar-specific tests from #1441 (basic, with_extras,
no_extras_vs_zero_extras, batched, batched_with_extras, matches_ase).
Existing tests are untouched.
djkim-kr pushed a commit to ChengUCB/mace that referenced this pull request May 14, 2026
Cherry-picks the polar-model functionality from ACEsuit#1441 without the
unrelated linter/style sweep:

- Auto-detect PolarMACE via isinstance and forward PolarMACE-specific
  inputs (rcell, volume, external_field, fermi_level, density_coefficients)
  with sensible defaults that match AtomicData.from_config().
- Read external_E_field from SimState extras when present.
- Expose PolarMACE outputs (charges, dipole, density_coefficients,
  polarizability, etc.) in the results dict.
- Pad the new tensors inline in _fill_padded_data, matching the
  existing displacement pattern, so the torch.compile path works for
  polar models without changing the function signature or introducing
  new persistent buffers.

Tests: add the 6 polar-specific tests from ACEsuit#1441 (basic, with_extras,
no_extras_vs_zero_extras, batched, batched_with_extras, matches_ase).
Existing tests are untouched.
Cherry-picks the polar-model functionality from ACEsuit#1441 without the
unrelated linter/style sweep:

- Auto-detect PolarMACE via isinstance and forward PolarMACE-specific
  inputs (rcell, volume, external_field, fermi_level, density_coefficients)
  with sensible defaults that match AtomicData.from_config().
- Read external_E_field from SimState extras when present.
- Expose PolarMACE outputs (charges, dipole, density_coefficients,
  polarizability, etc.) in the results dict.
- Pad the new tensors inline in _fill_padded_data, matching the
  existing displacement pattern, so the torch.compile path works for
  polar models without changing the function signature or introducing
  new persistent buffers.

Tests: add the 6 polar-specific tests from ACEsuit#1441 (basic, with_extras,
no_extras_vs_zero_extras, batched, batched_with_extras, matches_ase).
Existing tests are untouched.
@orionarcher
orionarcher force-pushed the orionarcher/torchsim-polarizability branch from ee0e3a7 to 6e118a6 Compare May 18, 2026 18:57
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