Skip to content

Commit d953ecd

Browse files
authored
feat: use flat_forms() for CDL v1.3 FormNode compatibility (#7)
Update geometry pipeline to use desc.flat_forms() instead of desc.forms for compatibility with CDL v1.3 FormGroup tree structure. Add 36 regression tests validating grouped vs ungrouped CDL produces identical geometry.
1 parent c7ef5db commit d953ecd

2 files changed

Lines changed: 104 additions & 3 deletions

File tree

src/crystal_geometry/geometry.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -314,7 +314,8 @@ def _build_halfspaces(
314314
face_form_indices = []
315315
face_millers = []
316316

317-
for form_idx, form in enumerate(desc.forms):
317+
forms = desc.flat_forms()
318+
for form_idx, form in enumerate(forms):
318319
miller = form.miller.as_3index()
319320
h, k, l = miller
320321

@@ -604,7 +605,7 @@ def _generate_twinned_geometry(
604605
face_normals=face_normals_list,
605606
face_forms=final_face_forms,
606607
face_millers=final_face_millers,
607-
forms=desc.forms,
608+
forms=desc.flat_forms(),
608609
component_ids=component_ids,
609610
twin_metadata=twin_metadata,
610611
)
@@ -650,7 +651,7 @@ def cdl_to_geometry(desc: CrystalDescription, c_ratio: float = 1.0) -> CrystalGe
650651
)
651652
else:
652653
geometry = _generate_base_geometry(
653-
normals, distances, face_form_indices, face_millers, desc.forms
654+
normals, distances, face_form_indices, face_millers, desc.flat_forms()
654655
)
655656
# Apply modifications if present
656657
if desc.modifications:

tests/test_geometry.py

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -405,3 +405,103 @@ def test_no_modifications(self):
405405
"""Test geometry without modifications."""
406406
geom = cdl_string_to_geometry("cubic[m3m]:{111}")
407407
assert geom.is_valid()
408+
409+
410+
# =============================================================================
411+
# CDL v1.3 flat_forms() Regression Tests
412+
# =============================================================================
413+
414+
415+
class TestFlatFormsRegression:
416+
"""Regression tests ensuring flat_forms() produces identical geometry.
417+
418+
Verifies that the geometry engine correctly uses flat_forms() for both
419+
v1-style CDL (plain form lists) and v1.3-style CDL (FormGroup trees).
420+
"""
421+
422+
# Mineral CDL strings covering all major crystal systems
423+
MINERAL_CDL_STRINGS = [
424+
# Cubic
425+
("diamond", "cubic[m3m]:{111}@1.0 + {100}@1.3"),
426+
("garnet", "cubic[m3m]:{110}@1.0 + {211}@0.6"),
427+
("fluorite", "cubic[m3m]:{111}"),
428+
("pyrite_cube", "cubic[m3m]:{100}"),
429+
("spinel", "cubic[m3m]:{111}@1.0"),
430+
# Hexagonal / Trigonal
431+
("quartz_prism", "hexagonal[6/mmm]:{10-10}@1.0 + {0001}@0.5"),
432+
("beryl", "hexagonal[6/mmm]:{10-10}@1.0 + {0001}@1.5"),
433+
# Tetragonal
434+
("zircon", "tetragonal[4/mmm]:{101}"),
435+
("rutile", "tetragonal[4/mmm]:{100}@1.0 + {101}@0.8"),
436+
# Orthorhombic
437+
("topaz", "orthorhombic[mmm]:{110}@1.0 + {001}@0.5"),
438+
("barite", "orthorhombic[mmm]:{001}@1.0 + {210}@0.8"),
439+
]
440+
441+
@pytest.mark.parametrize("name,cdl", MINERAL_CDL_STRINGS, ids=[m[0] for m in MINERAL_CDL_STRINGS])
442+
def test_mineral_geometry_valid(self, name, cdl):
443+
"""Each mineral CDL string produces valid geometry via flat_forms()."""
444+
geom = cdl_string_to_geometry(cdl)
445+
assert isinstance(geom, CrystalGeometry)
446+
assert len(geom.vertices) >= 4, f"{name}: too few vertices"
447+
assert len(geom.faces) >= 4, f"{name}: too few faces"
448+
assert all(len(f) >= 3 for f in geom.faces), f"{name}: degenerate face"
449+
450+
@pytest.mark.parametrize("name,cdl", MINERAL_CDL_STRINGS, ids=[m[0] for m in MINERAL_CDL_STRINGS])
451+
def test_mineral_geometry_euler(self, name, cdl):
452+
"""Euler characteristic V - E + F = 2 for all mineral geometries."""
453+
geom = cdl_string_to_geometry(cdl)
454+
assert geom.euler_characteristic() == 2, f"{name}: Euler != 2"
455+
456+
@pytest.mark.parametrize("name,cdl", MINERAL_CDL_STRINGS, ids=[m[0] for m in MINERAL_CDL_STRINGS])
457+
def test_mineral_geometry_forms_are_crystal_form(self, name, cdl):
458+
"""Geometry.forms contains only CrystalForm objects (not FormGroup)."""
459+
from cdl_parser import CrystalForm
460+
461+
geom = cdl_string_to_geometry(cdl)
462+
for form in geom.forms:
463+
assert isinstance(form, CrystalForm), (
464+
f"{name}: geometry.forms contains {type(form).__name__}, expected CrystalForm"
465+
)
466+
467+
def test_v13_grouped_cdl_geometry(self):
468+
"""v1.3 CDL with FormGroup produces valid geometry via flat_forms()."""
469+
cdl = "cubic[m3m]:({111}@1.0 + {100}@1.3)[phantom:3]"
470+
desc = parse_cdl(cdl)
471+
472+
# flat_forms() should return 2 CrystalForm objects with phantom feature
473+
flat = desc.flat_forms()
474+
assert len(flat) == 2
475+
assert all(hasattr(f, "miller") for f in flat)
476+
477+
# Geometry should be identical to the non-grouped version
478+
geom = cdl_to_geometry(desc)
479+
assert isinstance(geom, CrystalGeometry)
480+
assert len(geom.faces) == 14 # 8 octahedron + 6 cube
481+
assert geom.euler_characteristic() == 2
482+
483+
def test_v13_grouped_vs_ungrouped_identical(self):
484+
"""Grouped v1.3 CDL produces same vertex/face counts as ungrouped v1."""
485+
ungrouped = cdl_string_to_geometry("cubic[m3m]:{111}@1.0 + {100}@1.3")
486+
grouped = cdl_string_to_geometry("cubic[m3m]:({111}@1.0 + {100}@1.3)[phantom:3]")
487+
488+
assert len(ungrouped.vertices) == len(grouped.vertices)
489+
assert len(ungrouped.faces) == len(grouped.faces)
490+
assert ungrouped.euler_characteristic() == grouped.euler_characteristic()
491+
492+
def test_v1_cdl_flat_forms_identity(self):
493+
"""For v1-style CDL, flat_forms() returns identical forms to direct iteration."""
494+
from cdl_parser import CrystalForm
495+
496+
desc = parse_cdl("cubic[m3m]:{111}@1.0 + {100}@1.3")
497+
flat = desc.flat_forms()
498+
499+
# All should be CrystalForm
500+
assert len(flat) == 2
501+
assert all(isinstance(f, CrystalForm) for f in flat)
502+
503+
# Miller indices and scales preserved
504+
assert flat[0].miller.as_3index() == (1, 1, 1)
505+
assert flat[0].scale == 1.0
506+
assert flat[1].miller.as_3index() == (1, 0, 0)
507+
assert flat[1].scale == 1.3

0 commit comments

Comments
 (0)