Conversation
The Metal splat's test suite could pass while the kernel was dead code. _add_isotropic/_add_anisotropic swallowed any dispatch exception with `except Exception: pass`, so Engine.AUTO fell back to the same add_isotropic_plain_var that Engine.EAGER selects — every comparison then trivially agreed. The CUDA tests are safe from this only because Engine.TRITON re-raises; there was no equivalent for Metal. Engine.METAL + should_use_metal() - Fourth Engine member mirroring TRITON: forces the Metal path and raises rather than degrading (wrong device/dtype, uncompiled shader, or a dispatch failure). - should_use_metal() owns device, dtype and shader availability together. Folding availability in is what makes the force strict: as a nested `if` at the dispatch site, an unavailable shader slipped past the gate onto the portable splat even under a forced engine. - should_use_triton()'s implicit-AUTO tail is now explicit, with a ValueError fallthrough — that tail is why any new member silently selected Triton. - compile.last_error() is exported and consumed, so the raised message carries the compile diagnosis instead of being dead code. Kernel test coverage (both backends) - Cells, atom sets and metrics move to tests/helpers/kernel_cases.py; the two files had byte-identical private copies, which is how two gaps survived in both at once: unit occupancies hid the grad/occ rescaling, and zero off-diagonal U left every ellipsoid axis-aligned, so the cross-terms and the 4*pi^2 off-diagonal U gradients were never run. - Gradients now check magnitude, not just direction: assert_grads_agree moves to tests/helpers/grad_asserts.py. A bare cosine passes a kernel that returns 2*grad. - Dispatch provenance uses a call recorder, not map comparison: the portable splat's scatter_add is not order-reproducible on MPS (~1e-7 run to run), so torch.equal against it proves nothing either way. - Aniso now also runs on the sheared cell; only iso did before. - MPS forward tolerances re-measured (iso 5.9e-3, aniso 1.0e-3) and set from the measurements. CUDA tolerances left alone — retightening needs a CUDA host to measure on. Backend gating is the marker's job alone - --run-cuda/--run-mps warn and let the marked tests run, so they error with the real backend error. A collection-time UsageError aborted the whole session and said nothing about which call failed. - Removed 13 redundant availability checks (in-body skips and skipif decorators) sitting inside already-marked tests; a second layer can only mask a forgotten marker, turning "this host cannot run it" into a silent pass. Added cuda_device/mps_device fixtures that do no checking. - Promoted two tests mismarked @pytest.mark.gpu while hardcoding CUDA. ReflectionData: exempt non-per-reflection fields by name _reindex_per_reflection and __select__ decided "is this per-reflection?" by shape[0] == n_hkl. That collides when the dataset has exactly as many reflections as the field is long — U_aniso is (6,), so a 6-reflection dataset had it gathered and reordered as per-reflection data. _assert_per_reflection_consistent and reduce_to_spacegroup already exempted by name; all four are consistent now. epsilon_from_hkl: answer on hkl's device apply_to_hkl moves its input onto the symmetry matrices' device, so a spacegroup built elsewhere than the reflection data made the Hs == h0 comparison raise. Build h on the symmetry device, return on hkl's.
… and than used to confirm the the respective kernels.
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
1 participant
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.
validate_hkl/remap/reduce_to_spacegroup) now carries all per-reflection fields (including the anomalous bookkeeping read byhkl_for_sf()) instead of a hardcoded subset.