Magnetic potential/field improvements for collinear GPAW calculations - #326
Merged
Conversation
GPAW's new-style ASE calculator (default since GPAW 24, exclusive as of 26.x) exposes calc.parameters as a non-subscriptable Parameters dataclass instead of the old dict-like object, and renamed the density object's compensation charge coefficients from Q_aL to ccc_aL. Use attribute access for parameters and fall back to ccc_aL when Q_aL is absent, keeping compatibility with older/legacy GPAW calculators. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…r potential) The tile logic hard-coded the last 3 array axes as (z, x, y), which is only correct for scalar PotentialArray (_base_dims=3). For vector-valued fields like VectorPotentialArray and MagneticFieldArray (_base_dims=4, shape (..., z, 3, x, y)), this silently tiled the wrong axes (the 3-component axis instead of z), producing an array whose z-length no longer matched the tiled slice_thickness and raising "Number of slice thicknesses must match the number of slices." Build the tile repetition tuple from _base_dims so the z axis is always resolved at -_base_dims regardless of how many axes sit between it and x/y, fixing tile() for both scalar and vector-valued fields. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…on magnetic fields GPAWMagneticField/GPAWVectorPotential accepted frozen_phonons and repetitions parameters that were silently ignored: calculators was hard-asserted to a single GPAW instance despite the type hint allowing lists, and repetitions was never applied. Passing either produced silently wrong physics instead of an error. Now both raise NotImplementedError pointing at the supported workaround (tile() on the built array, and pairing a single-configuration magnetic field with a separate electrostatic FrozenPhonons ensemble). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…d default calculate_magnetic_vector_potential always builds the magnetization as m = (0, 0, rho): collinear spin has no real-space direction, so GPAW's internal spin axis is arbitrary. Because curl(m) and the subsequent Poisson solve are applied component-wise, this makes Az (the only component adjust_coulomb_potential uses) identically zero for every collinear calculation -- previously the user had to manually pass rotate_field to swap a nonzero in-plane component into z. rotate_field now defaults to "auto": it derives, from the raw computed field itself, the rotation that maximises the resulting z-component in a least-squares sense (closed-form via the 2x2 in-plane "power" matrix's principal axis, verified against brute-force angle search, rather than a fixed 90-degree swap that could land on the smaller of the two in-plane components. Explicit rotate_field tuples and rotate_field=None (raw, Az == 0 output) still work as before. The rotation matrix is applied directly instead of round-tripping through Euler angles, avoiding scipy's gimbal-lock warning on the degenerate (rotationally symmetric) cases where multiple angles are equally optimal. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> EOF )
Only x, y and z are physically meaningful directions here -- the orthogonal axes of the simulation cell -- since collinear spin has no inherent real-space direction. A continuous "optimal" blend of Ax and Ay (the previous least-squares approach) has no real-space interpretation as a magnetization direction; it would just fit whatever numerical asymmetry happens to be in the grid. Compare only the two 90-degree swaps (x into z, or y into z) and pick whichever has the larger aggregate magnitude. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…together Wraps GPAWPotential, GPAWVectorPotential and (optionally) GPAWMagneticField into a single call, returning a GPAWMagneticFields dataclass with .potential, .vector_potential and .magnetic_field. The magnetic components stay separate from the electrostatic one rather than being eagerly combined, since the frozen-phonon workflow requires combining after tiling: .tile() repeats the (rigid) magnetic components to match a separately built/tiled electrostatic potential (e.g. a CrystalPotential build from a FrozenPhonons ensemble), and .combined_potential() then folds the vector potential in via adjust_coulomb_potential. include_magnetic_field defaults to False since B is not used by combined_potential and roughly doubles the GPAW-side cost of the magnetic part. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Turns the notebook's manual make_potential_and_field_plots helper into a method on the bundle it already has all the data for: projects the potential and the x/z components of the vector potential (and, if built, the magnetic field) into a single ImageGrid figure. Panels for the magnetic field are only added when include_magnetic_field=True was used to build the bundle, since that component is optional. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
plt.figure() in interactive mode queues the figure for matplotlib_inline's automatic post-cell display; returning that same figure as the method's result then triggers a second, independent display via Jupyter's rich repr for the returned value. Creating the figure inside plt.ioff() (matching the same pattern already used in abtem.visualize.visualizations.Visualization) keeps matplotlib.is_interactive() False during figure creation, so matplotlib_inline's new_figure_manager_given_figure never queues it for the automatic flush -- only the returned Figure's own rich repr renders it, once. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
9 tasks
_DummyGPAW.from_gpaw fell back from Q_aL to ccc_aL whenever Q_aL was None, assuming that meant a new-style calculator. But on old-style GPAW, Density.Q_aL is initialized to None and only populated as a side effect of calculate_multipole_moments(), which runs during the SCF loop - not automatically on a plain restart via GPAW(path). So GPAWPotential(path, ...) (used by from_file) hit an AttributeError on ccc_aL, which doesn't exist on old-style density objects (confirmed by reading gpaw/old/pw/density.py's ReciprocalSpaceDensity, the exact class from the failing test's traceback). Fix: when neither Q_aL nor ccc_aL is already populated, compute it explicitly via calculate_multipole_moments() (old-style) or calculate_compensation_charge_coefficients() (new-style) instead of assuming one of the two attributes is always cached. Verified against local GPAW 26 (new-style, unaffected). Could not exercise the old-style restart path locally (blocked by an unrelated grid-size test failure present with or without this change); needs confirmation on the GPU machine that surfaced the original bug. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
# Conflicts: # abtem/potentials/gpaw.py
numerov() (abtem/inelastic/core_loss.py) allocated its output array with np.zeros(len(f)) inside an @jit(nopython=True) function. Same class of bug as the quasi-dipole interpolation fix (PR abTEM#332): some numba/numpy pairings (observed: numba 0.64.0 + numpy 2.4.3) fail to type numba's internal np.zeros -> np.empty lowering inside nopython mode, even for a plain scalar-int shape: TypingError: No implementation of function Function(<built-in function zeros>) ... Use of unsupported NumPy function 'numpy.empty' Fix: allocate via f.copy() instead. Every element of the output is overwritten before being read (x[0], x[1] directly, the rest via the loop), so the borrowed initial values from f are never used -- same pattern as the earlier fixes in pauli.py and iam.py. Surfaced by test_ionization.py::test_subshell_transitions_real_gpaw_pipeline on the same GPU workstation that found the two previous instances of this bug class. Verified: numerov's output is deterministic and unchanged (compared before/after on random input), and test_ionization.py passes locally (10 passed, 7 gpu-skipped); test_gpaw.py's failures on this machine are pre-existing and unrelated (grid-size mismatch, confirmed present before this change too). A full sweep of every np.zeros/np.empty/np.ones(_like) call inside @jit(nopython=True)/@njit-decorated functions elsewhere in the codebase found no other live instances -- integrals.py, measurements.py, and bloch/utils.py already follow the safe pattern (pre-allocate outside njit, pass in and mutate in place). abtem/magnetism/pauli.py's central_difference_gradient_pbc/_cbc have the same np.zeros_like pattern but are dead code (no callers outside the file, and their only internal caller apply_A_xy_dot_nabla_xy is itself uncalled), so numba's lazy compilation never reaches them -- left alone as a minor cleanup opportunity rather than bundled into this fix. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
FieldArray.tile()for vector-valued fields (VectorPotentialArray,MagneticFieldArray): it hard-coded the last 3 array axes as(z, x, y), which only holds for scalar potentials, so tiling a vector/magnetic field silently tiled the wrong axis and raised a slice-thickness mismatch error.GPAWMagneticField/GPAWVectorPotentialnow raiseNotImplementedErrorforfrozen_phonons/repetitionsinstead of silently ignoring them (they were accepted but never implemented).rotate_fieldnow defaults to"auto": since collinear spin has no real-space direction, GPAW's internal magnetization axis always producesAz == 0."auto"automatically swaps whichever in-plane component (AxorAy) has the larger magnitude intoz, restricted to the two physically meaningful 90-degree swaps (not an arbitrary blended angle). Explicitrotate_fieldtuples androtate_field=None(raw output) still work as before.gpaw_magnetic_fields(...), which builds the electrostatic potential, vector potential, and (optionally) magnetic field from the same GPAW calculator(s) in one call, returning aGPAWMagneticFieldsbundle with.tile()and.combined_potential()to support combining a rigid ab initio magnetic contribution with a separately tiled/frozen-phonon-ensembled electrostatic potential.GPAWMagneticFields.show()for a side-by-sideV/Ax/Az/Bx/Bzcomparison plot.Usage
Test plan
tile(), the auto rotation,gpaw_magnetic_fields(), andcombined_potential()are numerically correct/bit-identical to the equivalent manual API calls.GPAWMagneticFields.show()renders correctly (including the fix for it doubly-rendering in Jupyter).test/test_magnetics.py,test/test_potentials.pypass (42 passed, 18 skipped).🤖 Generated with Claude Code