feat: a CIF read warns when the operation loop and the Hall symbol disagree

Sources/DiffractoCore/CIFImport.swift

@@ -107,6 +107,22 @@ extension CIFImportError: LocalizedError {
     public var errorDescription: String? { description }
 }
 
+/// Something the import noticed and read past: the file is read, and the structure is the one described, but an app
+/// should show the warning beside it (`CIFDocument.warnings`).
+public enum CIFImportWarning: Sendable, Equatable, CustomStringConvertible {
+    /// The file lists an operation loop (one that passes the completeness checks) and states a Hall symbol whose group
+    /// is another one (compared modulo lattice translations). The loop was used, as pymatgen-core 2026.10.16 `CifParser` uses it; the Hall symbol was not.
+    case operationLoopIsNotTheHallGroup(hallSymbol: String, loopOrder: Int, hallOrder: Int)
+
+    public var description: String {
+        switch self {
+        case let .operationLoopIsNotTheHallGroup(hallSymbol, loopOrder, hallOrder):
+            return "The operation loop (\(loopOrder) operations) is not the group of the Hall symbol \(hallSymbol) "
+                + "(\(hallOrder) operations); the loop was used."
+        }
+    }
+}
+
 /// What a CIF read yields: the structure and what the file said about itself.
 public struct CIFDocument: Sendable, Equatable {
     public let structure: CrystalStructure
@@ -125,6 +141,8 @@ public struct CIFDocument: Sendable, Equatable {
     /// Whether the file carried expanding operations (`structure.fileOperations`): a symmetry-operation loop, or, with no
     /// loop, a Hall symbol read to its group. The structure's operations are detected from the expanded cell either way.
     public let hadSymmetryOperations: Bool
+    /// What the import read past (``CIFImportWarning``), in the order found; empty for a file with nothing to report.
+    public let warnings: [CIFImportWarning]
 }
 
 public enum CIFImport {
@@ -214,7 +232,8 @@ public enum CIFImport {
                            declaredSpaceGroupNumber: parsed.spaceGroupNumber,
                            declaredHallSymbol: parsed.hallSymbol,
                            coarsestHalfStep: parsed.coarsestHalfStep,
-                           hadSymmetryOperations: parsed.operations != nil)
+                           hadSymmetryOperations: parsed.operations != nil,
+                           warnings: parsed.warnings)
     }
 
     /// The name a structure imports under: the CIF's `data_` block name, unless it says nothing — empty, or the
@@ -240,6 +259,7 @@ public enum CIFImport {
         var spaceGroupNumber: Int?
         var spaceGroupSymbol: String?
         var hallSymbol: String?
+        var warnings: [CIFImportWarning]
     }
 
     /// One asymmetric-unit site plus how precisely each of its coordinates was written: `halfStep` is the half-width
@@ -420,6 +440,21 @@ public enum CIFImport {
         // A loop, even one holding only the identity, is what the file lists and wins over the symbol (as in pymatgen).
         let expandsWithHall = !haveSymmetryLoop && (hallOperations?.count ?? 0) > 1
         if expandsWithHall, let hallOperations { ops = hallOperations }
+        // A loop beside a readable Hall symbol of another group is used all the same (pymatgen reads no Hall symbol and
+        // uses the loop), but the reader is told. Equal groups: the same order and every listed operation in the Hall
+        // group, translations mod 1 to 1e-3 (the finest genuine translation is 1/12; a written 0.3333 is 1/3).
+        var warnings: [CIFImportWarning] = []
+        if fileListsOperations, let hallSymbol, let hallOperations {
+            let loopOrder = SymmetryOperation.distinctCount(ops)
+            let hallOrder = SymmetryOperation.distinctCount(hallOperations)
+            let sameGroup = loopOrder == hallOrder && ops.allSatisfy { op in
+                hallOperations.contains { $0.isEquivalent(to: op, translationTolerance: 1e-3) }
+            }
+            if !sameGroup {
+                warnings.append(.operationLoopIsNotTheHallGroup(hallSymbol: hallSymbol, loopOrder: loopOrder,
+                                                                hallOrder: hallOrder))
+            }
+        }
 
         let sites: [AtomSite]
         let operations: [SymmetryOperation]?
@@ -493,7 +528,7 @@ public enum CIFImport {
             a: a, b: b, c: c, alpha: alpha, beta: beta, gamma: gamma,
             sites: sites, operations: operations, coarsestHalfStep: coarsestHalfStep,
             spaceGroupNumber: spaceGroupNumber, spaceGroupSymbol: declaredHermannMauguin(cellValues),
-            hallSymbol: hallSymbol)
+            hallSymbol: hallSymbol, warnings: warnings)
     }
 
     // MARK: - Tokenizer

Sources/DiffractoCore/DiffractoCore.docc/DiffractoCore.md

@@ -26,6 +26,7 @@ It uses `Foundation` and `simd` only. Start with <doc:FromACIFToRings>.
 - ``CIFImport``
 - ``CIFDocument``
 - ``CIFImportError``
+- ``CIFImportWarning``
 
 ### Cells, sites and elements
 

Tests/DiffractoCoreTests/HallLoopWarningTests.swift

@@ -0,0 +1,91 @@
+//
+//  HallLoopWarningTests.swift
+//  A CIF that lists a full operation loop and also states a Hall symbol of another group: the loop is used, as
+//  pymatgen-core 2026.10.16 `CifParser` uses it (it reads no Hall symbol), and the document carries a warning that names
+//  the symbol and the two orders. The groups are spglib 2.8.0's (`hall-symbol-pins.json`).
+//
+
+import DiffractoKitTestSupport
+import Foundation
+import Testing
+@testable import DiffractoCore
+
+struct HallLoopWarningTests {
+    static func cif(hall: String?, ops: [String]) -> String {
+        var s = "data_t\n_symmetry_Int_Tables_number 123\n"
+        if let hall { s += "_space_group_name_Hall '\(hall)'\n" }
+        for (tag, v) in zip(["length_a", "length_b", "length_c", "angle_alpha", "angle_beta", "angle_gamma"],
+                            [10.0, 10.0, 12.0, 90, 90, 90]) {
+            s += "_cell_\(tag) \(v)\n"
+        }
+        s += "loop_\n_symmetry_equiv_pos_as_xyz\n" + ops.map { "'\($0)'" }.joined(separator: "\n") + "\n"
+        s += "loop_\n_atom_site_label\n_atom_site_type_symbol\n_atom_site_fract_x\n_atom_site_fract_y\n_atom_site_fract_z\n"
+        s += "Si1 Si 0.1 0.2 0.3\n"
+        return s
+    }
+
+    /// A P4/mmm loop beside the Hall symbol of mmm (-P 2 2): the 16 images of a general site are read from the loop, and
+    /// one warning names the symbol and the orders 16 and 8. RED UNDER: `CIFDocument` had no warnings.
+    @Test func aLoopOfAnotherGroupThanTheHallSymbolIsUsedWithAWarning() throws {
+        let loop = try OperationListCheckTests.group("-P 4 2").map(\.xyzString)
+        #expect(loop.count == 16)
+        let document = try CIFImport.read(Self.cif(hall: "-P 2 2", ops: loop))
+        #expect(document.structure.sites.count == 16)
+        #expect(document.warnings == [.operationLoopIsNotTheHallGroup(hallSymbol: "-P 2 2", loopOrder: 16, hallOrder: 8)])
+        let text = try #require(document.warnings.first).description
+        #expect(text.contains("-P 2 2") && text.contains("16") && text.contains("8") && text.contains("loop was used"))
+    }
+
+    /// Equal orders are not enough: a P4/mmm loop (16 operations) beside the Hall symbol of P4/mcc (-P 4 2c, also 16
+    /// operations, the same crystal class) warns, because the c-glide operations are not in the loop.
+    @Test func aLoopOfTheSameOrderButAnotherGroupWarns() throws {
+        let loop = try OperationListCheckTests.group("-P 4 2").map(\.xyzString)
+        #expect(try OperationListCheckTests.group("-P 4 2c").count == 16)
+        let document = try CIFImport.read(Self.cif(hall: "-P 4 2c", ops: loop))
+        #expect(document.structure.sites.count == 16)
+        #expect(document.warnings == [.operationLoopIsNotTheHallGroup(hallSymbol: "-P 4 2c", loopOrder: 16, hallOrder: 16)])
+    }
+
+    /// A Hall symbol that cannot be read beside a valid loop: the loop is used, nothing is thrown and nothing is warned.
+    @Test func anUnreadableHallSymbolBesideALoopHasNoWarning() throws {
+        let loop = try OperationListCheckTests.group("-P 4 2").map(\.xyzString)
+        let document = try CIFImport.read(Self.cif(hall: "Q 9 9", ops: loop))
+        #expect(document.structure.sites.count == 16)
+        #expect(document.warnings.isEmpty)
+    }
+
+    /// The same loop beside its own Hall symbol (-P 4 2), and beside none: no warning, the same 16 sites.
+    @Test func aLoopOfTheHallGroupOrWithoutAHallSymbolHasNoWarning() throws {
+        let loop = try OperationListCheckTests.group("-P 4 2").map(\.xyzString)
+        for hall in ["-P 4 2", nil] as [String?] {
+            let document = try CIFImport.read(Self.cif(hall: hall, ops: loop))
+            #expect(document.structure.sites.count == 16)
+            #expect(document.warnings.isEmpty, "\(hall ?? "no Hall symbol")")
+        }
+    }
+
+    /// The loop written in reverse order, with every translation moved by a lattice vector, is still the Hall group: the
+    /// comparison is of sets, modulo lattice translations.
+    @Test func theComparisonIgnoresOrderAndLatticeTranslations() throws {
+        let ops = try OperationListCheckTests.group("-P 4 2")
+        let shifted = ops.reversed().map { op in
+            SymmetryOperation(rotation: op.rotation, translation: op.translation + SIMD3(1, 0, -1)).xyzString
+        }
+        let document = try CIFImport.read(Self.cif(hall: "-P 4 2", ops: shifted))
+        #expect(document.warnings.isEmpty)
+    }
+
+    /// Every file read without a Hall symbol, or with only a Hall symbol, has no warning.
+    @Test func aHallOnlyFileHasNoWarning() throws {
+        var s = "data_t\n_space_group_name_Hall '-P 4 2'\n"
+        for (tag, v) in zip(["length_a", "length_b", "length_c", "angle_alpha", "angle_beta", "angle_gamma"],
+                            [10.0, 10.0, 12.0, 90, 90, 90]) {
+            s += "_cell_\(tag) \(v)\n"
+        }
+        s += "loop_\n_atom_site_label\n_atom_site_type_symbol\n_atom_site_fract_x\n_atom_site_fract_y\n_atom_site_fract_z\n"
+        s += "Si1 Si 0.1 0.2 0.3\n"
+        let document = try CIFImport.read(s)
+        #expect(document.structure.sites.count == 16)
+        #expect(document.warnings.isEmpty)
+    }
+}

docs/parity.md

@@ -178,7 +178,7 @@ matching and phase mapping of DiffractoOrientation, as a pinned f050d207 environ
 | Python function or option group | DiffractoKit symbol | State | Plan | Upstream tolerance (test file) | How checked |
 |---|---|---|---|---|---|
 | `CifParser.from_str(...).parse_structures()` | `CIFImport.read(_:fileBaseName:)` -> `CIFDocument.structure` | **partly** | study | tests not shipped; function arguments `occupancy_tolerance=1.0`, `site_tolerance=1e-4`, `frac_tolerance=1e-4`, `comp_tol=0.01` | P2 on 19 structures: cell, expanded sites, operations equal. Differences by design: the kit reads the subset that defines a structure (cell, atom-site loop, operation loop), refuses what it cannot expand safely (named errors, `CIFImportError`), merges images within the written precision and 0.10 A (pymatgen: `site_tolerance` 1e-4; the kit is looser by design) and detects the symmetry from the expanded cell; no file is read from disk (the caller passes text) |
-| symmetry of a CIF that names a group but lists no operations (`get_symops` falling back to `SpaceGroup`), and a group named by Hermann-Mauguin symbol alone | `CIFDocument.declaredSpaceGroupSymbol`, `declaredSpaceGroupNumber`, `declaredHallSymbol`, `CIFImport.spaceGroupOrder`; `hadSymmetryOperations` | **partly** | study | tests not shipped; assert_allclose defaults | the completeness check works from the IT number, else the Hermann-Mauguin symbol (P35), else the Hall symbol (P37). A Hall symbol with no loop is expanded with its own group (P37: the spglib atom count; pymatgen builds P1 there); a number or Hermann-Mauguin symbol with no loop is refused (no tables). A listed list must also map the cell onto itself, be closed and have the IT number's crystal class (`CIFImportError.inconsistentSymmetryOperations`); pymatgen checks none of these. **DEVIATION**: pymatgen-core 2026.10.16 `CifParser` admits a listed operation list that does not map the cell onto itself, is not closed, or has another crystal class than the IT number, and expands the atom sites with it as given; the kit refuses such a file, and the error names the cause in words (`SymmetryListProblem`: the operation that does not map the cell onto itself, the product the list lacks, or the two crystal classes). A full loop that disagrees with the Hall symbol beside it is used, as pymatgen does |
+| symmetry of a CIF that names a group but lists no operations (`get_symops` falling back to `SpaceGroup`), and a group named by Hermann-Mauguin symbol alone | `CIFDocument.declaredSpaceGroupSymbol`, `declaredSpaceGroupNumber`, `declaredHallSymbol`, `CIFImport.spaceGroupOrder`; `hadSymmetryOperations` | **partly** | study | tests not shipped; assert_allclose defaults | the completeness check works from the IT number, else the Hermann-Mauguin symbol (P35), else the Hall symbol (P37). A Hall symbol with no loop is expanded with its own group (P37: the spglib atom count; pymatgen builds P1 there); a number or Hermann-Mauguin symbol with no loop is refused (no tables). A listed list must also map the cell onto itself, be closed and have the IT number's crystal class (`CIFImportError.inconsistentSymmetryOperations`); pymatgen checks none of these. **DEVIATION**: pymatgen-core 2026.10.16 `CifParser` admits a listed operation list that does not map the cell onto itself, is not closed, or has another crystal class than the IT number, and expands the atom sites with it as given; the kit refuses such a file, and the error names the cause in words (`SymmetryListProblem`: the operation that does not map the cell onto itself, the product the list lacks, or the two crystal classes). A loop that passes those checks and disagrees with the Hall symbol beside it is used, as pymatgen does; pymatgen says nothing, the kit adds `CIFImportWarning.operationLoopIsNotTheHallGroup` (the symbol and both orders) to `CIFDocument.warnings` (`HallLoopWarningTests`: a P4/mmm loop beside -P 2 2 or -P 4 2c warns, beside -P 4 2 or an unreadable symbol does not) |
 | `CifParser(filename)`, `CifFile.from_file`, `CifBlock` | none | not held | not planned (`CIFImport` takes text; the caller reads files) | tests not shipped; assert_allclose defaults |  |
 | `CifParser.get_symops` / `SymmOp.from_xyz_str`, `as_xyz_str` | `SymmetryOperation(xyz:)`, `xyzString` | **partly** | study | tests not shipped; assert_allclose defaults | P2 builds the pinned operations from pymatgen's strings; the grammar is mac4DSTEM 7d6fdbd1's with a refusal of singular rotations added (`SymmetryOperationTests`); `xyzString` is not compared with `as_xyz_str` |
 | `MPRester` summary documents (`structure`, `symmetry`) as a source of structures | `MaterialsProjectImport` (`decode`, `structure(from:)`, `check(_:against:)`), `MaterialsProjectDocument` | **partly** | useful | tests not shipped; assert_allclose defaults | P20, P30: the conventional cell built from the committed MP-shaped inputs equals pymatgen's `SpacegroupAnalyzer` conventional cell (volume 1e-7); the request builder is not run against the service. MP and ICSD database access beyond the request builder is not planned |