From b0508ded667352d2da8f31403bc5bc9fcb3185f9 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 26 Oct 2026 00:25:17 +0000 Subject: [PATCH 1/4] Keep ionic lattice ball centres inside the conventional cell Extended ionic and inorganic unit-cell views were drawing a one-sided coordination shell, such as NaCl Cl at fractional -0.5, from spanning-bond context. Display those lattices as wrapped crystallographic sites plus symmetric 0/1 boundary images. Co-authored-by: Ming-Yu Guo --- docs/agents/scene_api.md | 10 + mat_viewer/render/display_modes.py | 87 +++++++- mat_viewer/scene/core.py | 25 ++- tests/render/test_nacl_ball_unit_cell.py | 244 +++++++++++++++++++++++ tests/test_inorganic_defaults.py | 10 +- 5 files changed, 365 insertions(+), 11 deletions(-) create mode 100644 tests/render/test_nacl_ball_unit_cell.py diff --git a/docs/agents/scene_api.md b/docs/agents/scene_api.md index 2ba1cd5d..1191582e 100644 --- a/docs/agents/scene_api.md +++ b/docs/agents/scene_api.md @@ -84,6 +84,9 @@ Parses a CIF and returns a scene dict consumable by Per-species counts come from MolCrysKit's `StoichiometryAnalyzer.get_simplest_unit()` (unit-cell species counts divided by their GCD), not MatterVis-local hard-coded heuristics. + Extended ionic and inorganic solids do not keep that molecular unwrap: + displayed formula-unit centres are folded into the home cell so a cell + overlay does not show a one-sided exterior halo. - `unit_cell` — every atom of the conventional cell, with PBC bond imaging. When MolCrysKit canonical bond records are available, every manifested boundary-fragment image receives the corresponding canonical @@ -94,6 +97,13 @@ Parses a CIF and returns a scene dict consumable by triggers an image, the complete molecule is translated. Image shifts are unioned per member; face signals from different members are never combined into an unsupported diagonal image. + Extended ionic and inorganic solids use a different unit-cell contract. + Each crystallographic site is wrapped into the home cell and, when + boundary replicas are on, copied to the symmetric face, edge, and corner + images at 0 and 1. Molecular formula-unit unwrap and spanning-bond + context images (the rocksalt shell at fractional `-0.5`) are not drawn. + With boundary replicas off, the same lattices stay on the wrapped sites + and do not grow a bonded-image shell outside the cell. Library callers that need a strict publication cell can pass `include_boundary_replicas=False` to `build_scene_from_atoms` or `build_bundle_scene`. Atom centres are wrapped into `[0, 1)` and no adjacent diff --git a/mat_viewer/render/display_modes.py b/mat_viewer/render/display_modes.py index 7606d0c1..f1bd7474 100644 --- a/mat_viewer/render/display_modes.py +++ b/mat_viewer/render/display_modes.py @@ -14,6 +14,63 @@ import numpy as np from molcrys_kit.utils.geometry import frac_to_cart +# Snap values that ``np.mod`` leaves on the 1-boundary back onto 0 so a +# crystallographic site is not stored twice, once at each end of the axis. +_WRAP_ONE_ATOL = 1e-9 + + +def _wrap_fractional(frac: Any) -> np.ndarray | None: + """Fold one fractional coordinate into the home cell, with 1 identified as 0.""" + arr = np.asarray(frac, dtype=float) + if arr.shape != (3,) or not np.all(np.isfinite(arr)): + return None + wrapped = np.mod(arr, 1.0) + near_zero = np.isclose(wrapped, 0.0, rtol=0.0, atol=_WRAP_ONE_ATOL) + near_one = np.isclose(wrapped, 1.0, rtol=0.0, atol=_WRAP_ONE_ATOL) + if np.any(near_zero) or np.any(near_one): + wrapped = np.array(wrapped, dtype=float, copy=True) + wrapped[near_zero | near_one] = 0.0 + return wrapped + + +def _independent_wrapped_sites( + atoms: list[dict[str, Any]], + M: Any, +) -> list[dict[str, Any]]: + """Return one wrapped crystallographic site per input atom. + + Ionic and inorganic lattices are not finite molecules. Keeping the + MolCrysKit molecule index would union every member's face shift and + translate the whole framework, and a non-zero periodic rank would + suppress those face images in favour of a one-sided coordination + shell. Each site is therefore replicated on its own. + """ + M_arr = np.asarray(M, dtype=float) + out: list[dict[str, Any]] = [] + for fallback_index, atom in enumerate(atoms): + frac = _wrap_fractional(atom.get("frac")) + if frac is None: + continue + copied = dict(atom) + copied["frac"] = frac + copied["cart"] = frac_to_cart(frac, M_arr) + copied["_wrapped_frac"] = np.array(frac, dtype=float, copy=True) + copied["_image_shift"] = (0, 0, 0) + copied.setdefault("_source_index", fallback_index) + for key in ( + "_source_molecule_index", + "_periodic_component_rank", + "_unwrapped", + "_cell_spanning_component", + "_is_boundary_replica", + "_is_fragment_boundary_replica", + "_is_framework_context_replica", + "_is_bonded_image_replica", + ): + copied.pop(key, None) + out.append(copied) + return out + def _asymmetric_unit_atoms(atoms: list[dict[str, Any]]) -> list[dict[str, Any]]: """Deduplicate atoms by label + element + disorder tags.""" @@ -42,11 +99,9 @@ def _wrapped_unit_cell_atoms( out: list[dict[str, Any]] = [] for source_index, atom in enumerate(atoms): source_frac = atom.get("_wrapped_frac", atom.get("frac")) - frac = np.asarray(source_frac, dtype=float) - if frac.shape != (3,) or not np.all(np.isfinite(frac)): + wrapped = _wrap_fractional(source_frac) + if wrapped is None: continue - wrapped = np.mod(frac, 1.0) - wrapped[np.isclose(wrapped, 1.0, rtol=0.0, atol=1e-9)] = 0.0 copied = dict(atom) copied["frac"] = wrapped copied["cart"] = frac_to_cart(wrapped, M_arr) @@ -69,6 +124,7 @@ def selected_atoms_for_mode( formula_unit_atoms: list[dict[str, Any]] | None = None, unwrapped_atoms: list[dict[str, Any]] | None = None, include_boundary_replicas: bool = True, + lattice_site_display: bool = False, ) -> list[dict[str, Any]]: """Return the drawable atom list for the given display mode. @@ -78,11 +134,24 @@ def selected_atoms_for_mode( - ``"unit_cell"`` — conventional cell with boundary replicas - ``"asymmetric_unit"`` — only the asymmetric unit - ``"cluster"`` — every parsed atom, no PBC processing + + ``lattice_site_display`` is set for extended ionic and inorganic + solids. Those views use wrapped crystallographic sites. Unit-cell + replicas are the symmetric 0/1 face, edge, and corner images of each + site. Formula-unit centres are folded into the home cell instead of + keeping a molecular unwrap that sits outside the box. """ from .boundary_replicas import expand_boundary_replicas continuous_atoms = unwrapped_atoms if unwrapped_atoms else atoms if display_mode == "unit_cell": + if lattice_site_display: + sites = _independent_wrapped_sites(atoms, M) + if not include_boundary_replicas: + for site in sites: + site["_strict_unit_cell"] = True + return sites + return expand_boundary_replicas(sites, M) if not include_boundary_replicas: return _wrapped_unit_cell_atoms(atoms, M) base = [dict(atom) for atom in continuous_atoms] @@ -93,6 +162,10 @@ def selected_atoms_for_mode( return [dict(atom) for atom in atoms] # formula_unit: defer to MolCrysKit if formula_unit_atoms is not None: - return [dict(atom) for atom in formula_unit_atoms] - from mat_viewer.structure import molcrys_bridge - return molcrys_bridge.select_formula_unit(atoms, M) + chosen = [dict(atom) for atom in formula_unit_atoms] + else: + from mat_viewer.structure import molcrys_bridge + chosen = molcrys_bridge.select_formula_unit(atoms, M) + if lattice_site_display: + return _independent_wrapped_sites(chosen, M) + return chosen diff --git a/mat_viewer/scene/core.py b/mat_viewer/scene/core.py index 2f70737f..3118bd9e 100644 --- a/mat_viewer/scene/core.py +++ b/mat_viewer/scene/core.py @@ -249,6 +249,14 @@ def _formula_signature(items): ) formula_unit_atoms = canonical_formula_atoms + # Extended ionic / inorganic solids are lattices, not molecules. A + # formula-unit unwrap and a spanning-bond shell both park rocksalt + # sites at fractional coordinates such as -0.5, which draws a + # one-sided halo outside the conventional cell. + lattice_site_display = display_mode in {"formula_unit", "unit_cell"} and ( + bool(ionic_source_pairs) + or bool(inorganic_profile.get("is_inorganic_solid")) + ) sel_atoms = selected_atoms_for_mode( ops, source_atoms, @@ -258,6 +266,7 @@ def _formula_signature(items): formula_unit_atoms=formula_unit_atoms, unwrapped_atoms=canonical_unwrapped_atoms, include_boundary_replicas=include_boundary_replicas, + lattice_site_display=lattice_site_display, ) draw_atoms = [dict(atom) for atom in sel_atoms if show_h or atom["elem"] != "H"] if not include_minor: @@ -267,14 +276,26 @@ def _formula_signature(items): strict_cell = display_mode == "unit_cell" and not include_boundary_replicas bonded_image_replica_count = 0 framework_context_replica_count = 0 - if strict_cell and image_records and include_cross_boundary_bond_endpoints: + # Lattice sites already carry their symmetric boundary images. Adding + # signed coordination endpoints here rebuilds the exterior halo + # (strict-cell NaCl grows to the full neighbour shell outside [0, 1]). + if ( + not lattice_site_display + and strict_cell + and image_records + and include_cross_boundary_bond_endpoints + ): bonded_image_replica_count = _manifest_strict_bonded_images( draw_atoms, source_atoms, M, image_records, ) - elif display_mode == "unit_cell" and include_boundary_replicas: + elif ( + not lattice_site_display + and display_mode == "unit_cell" + and include_boundary_replicas + ): framework_context_replica_count = _manifest_spanning_bond_context( draw_atoms, source_atoms, diff --git a/tests/render/test_nacl_ball_unit_cell.py b/tests/render/test_nacl_ball_unit_cell.py new file mode 100644 index 00000000..56540acd --- /dev/null +++ b/tests/render/test_nacl_ball_unit_cell.py @@ -0,0 +1,244 @@ +"""Cubic rocksalt ball view stays inside the conventional cell. + +``--style ball`` draws no sticks. A unit-cell figure is the wrapped +crystallographic sites plus, when boundary replicas are on, the symmetric +face, edge, and corner images at 0 and 1. A one-sided coordination shell +at fractional ``-0.5`` is not part of that figure. +""" + +from __future__ import annotations + +import argparse +from itertools import permutations +from pathlib import Path + +import numpy as np +from ase.build import bulk +from ase.io import write + +from mat_viewer.cli import _display_mode +from mat_viewer.loader import build_bundle_scene +from mat_viewer.loader.structure_input import load_structure_input +from mat_viewer.render.contracts import RenderSpec, ViewSpec +from mat_viewer.render.planning import prepare_render + +_FRAC_DECIMALS = 6 + + +def _poscar(tmp_path: Path) -> Path: + atoms = bulk("NaCl", "rocksalt", a=5.6402, cubic=True) + path = tmp_path / "POSCAR_NaCl" + write(path, atoms, format="vasp", direct=True, vasp5=True) + return path + + +def _scene(path: Path, *, replicas: bool, cross_boundary: bool, mode: str = "unit_cell"): + loaded = load_structure_input(path) + return loaded, build_bundle_scene( + loaded.frames[0].bundle, + display_mode=mode, + show_hydrogen=True, + include_boundary_replicas=replicas, + include_cross_boundary_bond_endpoints=cross_boundary, + ) + + +def _site_key(element: str, frac) -> tuple[str, tuple[float, float, float]]: + rounded = np.round(np.asarray(frac, dtype=float), _FRAC_DECIMALS) + rounded[np.abs(rounded) < 10 ** (-_FRAC_DECIMALS)] = 0.0 + return element, tuple(float(value) for value in rounded) + + +def _draw_keys(scene) -> list[tuple[str, tuple[float, float, float]]]: + return [ + _site_key(str(atom["elem"]), atom["frac"]) + for atom in scene["draw_atoms"] + ] + + +def _assert_inside_closed_cell(keys) -> None: + for _element, frac in keys: + assert all(-1e-8 <= component <= 1.0 + 1e-8 for component in frac) + + +def _boundary_images(frac: tuple[float, float, float]) -> list[tuple[float, float, float]]: + """Symmetric 0/1 images of one conventional-cell special position.""" + axes: list[tuple[float, ...]] = [] + for value in frac: + if abs(value) <= 1e-8 or abs(value - 1.0) <= 1e-8: + axes.append((0.0, 1.0)) + else: + axes.append((float(value),)) + return [ + (x, y, z) + for x in axes[0] + for y in axes[1] + for z in axes[2] + ] + + +def _expected_replica_keys() -> list[tuple[str, tuple[float, float, float]]]: + sodium = ( + (0.0, 0.0, 0.0), + (0.5, 0.5, 0.0), + (0.5, 0.0, 0.5), + (0.0, 0.5, 0.5), + ) + chlorine = ( + (0.5, 0.0, 0.0), + (0.0, 0.5, 0.0), + (0.0, 0.0, 0.5), + (0.5, 0.5, 0.5), + ) + keys = [] + for element, sites in (("Na", sodium), ("Cl", chlorine)): + for site in sites: + keys.extend( + _site_key(element, image) for image in _boundary_images(site) + ) + return keys + + +def _assert_cubic_symmetry(keys) -> None: + present = set(keys) + for element, frac in keys: + inverted = _site_key(element, tuple(1.0 - value for value in frac)) + assert inverted in present + for order in permutations(range(3)): + permuted = _site_key(element, tuple(frac[axis] for axis in order)) + assert permuted in present + + +def test_auto_view_of_periodic_poscar_is_unit_cell(tmp_path: Path) -> None: + loaded, _scene_unused = _scene( + _poscar(tmp_path), + replicas=True, + cross_boundary=False, + ) + assert _display_mode(loaded, argparse.Namespace(view="auto")) == "unit_cell" + + +def test_ball_unit_cell_has_symmetric_boundary_images_only(tmp_path: Path) -> None: + """ASE cubic NaCl ``--style ball --view unit_cell``. + + Boundary replicas default on. Centres stay in ``[0, 1]``, including the + intentional images that sit on a face, edge, or corner. Inversion through + the cell centre and cubic axis permutation both preserve the figure. + """ + path = _poscar(tmp_path) + _loaded, scene = _scene(path, replicas=True, cross_boundary=False) + keys = _draw_keys(scene) + + assert scene["display_mode"] == "unit_cell" + assert scene["is_ionic_solid"] is True + assert scene["is_inorganic_solid"] is True + assert scene["bonds"] == [] + assert scene["framework_context_replica_count"] == 0 + assert scene["bonded_image_replica_count"] == 0 + assert not any( + atom.get("_is_framework_context_replica") or atom.get("_is_bonded_image_replica") + for atom in scene["draw_atoms"] + ) + for atom in scene["draw_atoms"]: + shift = tuple(int(value) for value in atom.get("_image_shift", (0, 0, 0))) + assert shift in {(x, y, z) for x in (0, 1) for y in (0, 1) for z in (0, 1)} + + _assert_inside_closed_cell(keys) + _assert_cubic_symmetry(keys) + assert sorted(keys) == sorted(_expected_replica_keys()) + assert len(keys) == 27 + + loaded = load_structure_input(path) + plan = prepare_render( + loaded, + view=ViewSpec(display="unit_cell", include_boundary_replicas=True), + render=RenderSpec( + representation="ball", + backend="cpu", + show_cell=True, + show_axes=False, + show_labels=False, + shading="flat", + ), + ) + matrix = np.asarray(loaded.frames[0].bundle.M, dtype=float) + centres = [ + np.asarray(primitive.metadata["_raster_center"], dtype=float) + for primitive in plan.primitives + if primitive.metadata.get("kind") == "atom" + ] + assert len(centres) == len(keys) + fracs = np.asarray(centres, dtype=float) @ np.linalg.inv(matrix) + assert np.all(fracs >= -1e-6) + assert np.all(fracs <= 1.0 + 1e-6) + rendered = [ + _site_key(str(primitive.metadata["element"]), frac) + for primitive, frac in zip( + ( + primitive + for primitive in plan.primitives + if primitive.metadata.get("kind") == "atom" + ), + fracs, + ) + ] + assert sorted(rendered) == sorted(keys) + + +def test_ball_unit_cell_without_replicas_stays_on_wrapped_sites(tmp_path: Path) -> None: + """``--no-boundary-replicas`` must not grow the bonded-image halo.""" + path = _poscar(tmp_path) + expected = [ + _site_key(element, frac) + for element, sites in ( + ( + "Na", + ( + (0.0, 0.0, 0.0), + (0.5, 0.5, 0.0), + (0.5, 0.0, 0.5), + (0.0, 0.5, 0.5), + ), + ), + ( + "Cl", + ( + (0.5, 0.0, 0.0), + (0.0, 0.5, 0.0), + (0.0, 0.0, 0.5), + (0.5, 0.5, 0.5), + ), + ), + ) + for frac in sites + ] + for cross_boundary in (False, True): + _loaded, scene = _scene( + path, + replicas=False, + cross_boundary=cross_boundary, + ) + keys = _draw_keys(scene) + assert len(scene["draw_atoms"]) == 8 + assert scene["bonded_image_replica_count"] == 0 + assert scene["bonds"] == [] + _assert_inside_closed_cell(keys) + assert sorted(keys) == sorted(expected) + + +def test_ionic_formula_unit_centres_stay_in_the_home_cell(tmp_path: Path) -> None: + _loaded, scene = _scene( + _poscar(tmp_path), + replicas=True, + cross_boundary=False, + mode="formula_unit", + ) + keys = _draw_keys(scene) + assert scene["bonds"] == [] + assert keys + _assert_inside_closed_cell(keys) + assert all( + not atom.get("_is_boundary_replica") + and not atom.get("_is_framework_context_replica") + for atom in scene["draw_atoms"] + ) diff --git a/tests/test_inorganic_defaults.py b/tests/test_inorganic_defaults.py index 158a4b9b..0e2ea74c 100644 --- a/tests/test_inorganic_defaults.py +++ b/tests/test_inorganic_defaults.py @@ -117,10 +117,16 @@ def test_nacl_keeps_mck_contact_but_hides_ionic_sticks(): show_hydrogen=True, preset={}, ) - # mck 0.8.0: spanning 1-D contact graph -> framework replicas (4), not corner expansion (16) - assert len(unit_cell["draw_atoms"]) == 4 + # Origin Na is a corner site (8 images) and Cl at (0.5, 0, 0) is an + # edge site (4 images). Symmetric 0/1 copies stay inside the cell; + # the old spanning-bond shell (4 atoms, including Cl at -0.5) does not. + assert len(unit_cell["draw_atoms"]) == 12 + assert unit_cell["framework_context_replica_count"] == 0 assert unit_cell["bonds"] == [] assert unit_cell["suppressed_ionic_bond_count"] > 0 + for atom in unit_cell["draw_atoms"]: + frac = atom["frac"] + assert all(-1e-8 <= float(component) <= 1.0 + 1e-8 for component in frac) def test_binary_ionic_default_state_uses_cell_and_atom_coordination(tmp_path: Path): From 7b4ffb73991b9c68a9947095a7774c69578856db Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 26 Oct 2026 00:28:55 +0000 Subject: [PATCH 2/4] Draw complete rocksalt coordination sticks for ball-and-stick Binary alkali-halide contacts stay in the visible bond list, and a half-cell neighbour keeps its manifested endpoint so each site shows all six Fm-3m arms. An omitted CLI --style follows the viewer lattice default of ball on the unit cell. Co-authored-by: Ming-Yu Guo --- docs/agents/scene_api.md | 6 + docs/cli.md | 2 +- mat_viewer/app/backend_core.py | 5 +- mat_viewer/cli.py | 38 ++++ mat_viewer/render/cli.py | 8 +- mat_viewer/render/planning.py | 29 ++- mat_viewer/render/radius.py | 33 ++- mat_viewer/scene/core.py | 26 +-- mat_viewer/scene/periodic.py | 39 ++-- mat_viewer/structure/bonds.py | 52 +++-- mat_viewer/transforms/core.py | 12 +- skills/visualize-materials/SKILL.md | 4 +- .../references/polyhedra.md | 6 +- .../references/quickstart.md | 7 +- .../render/test_ionic_coordination_sticks.py | 192 ++++++++++++++++++ tests/test_inorganic_defaults.py | 19 +- 16 files changed, 390 insertions(+), 88 deletions(-) create mode 100644 tests/render/test_ionic_coordination_sticks.py diff --git a/docs/agents/scene_api.md b/docs/agents/scene_api.md index 1191582e..30865f08 100644 --- a/docs/agents/scene_api.md +++ b/docs/agents/scene_api.md @@ -115,6 +115,12 @@ Parses a CIF and returns a scene dict consumable by `include_boundary_replicas=False` and `include_cross_boundary_bond_endpoints=False`. It is display-only. Source fractional coordinates on the loaded bundle stay unchanged. + Binary alkali and alkaline-earth halide contacts stay in `scene["bonds"]`. + An explicit ball-and-stick style draws that complete first-neighbor + coordination network, including the periodic images required for the + lattice symmetry. Ball and space-filling renderers do not stroke those + contacts. When `--style` is omitted, an ionic or inorganic lattice uses + `ball` and `unit_cell`, the same default as the viewer. - `asymmetric_unit` — only the asymmetric unit is drawn. - `cluster` — **free molecular cluster mode**. Every parsed atom is drawn unchanged; no formula-unit selection or periodic image diff --git a/docs/cli.md b/docs/cli.md index b36bc8b8..3a7ade83 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -298,7 +298,7 @@ substitutes one frame backend for another. | Flag | Default | Description | |------|---------|-------------| | --view MODE | auto | Periodic input uses unit_cell; nonperiodic or synthetic-cell input uses cluster | -| --style STYLE | ball_stick | ball_stick, ball, space_filling, stick, ortep, or wireframe | +| --style STYLE | auto | Omitted: `ball_stick` for molecules and covalent networks; `ball` plus the unit cell for ionic and inorganic lattices. Explicit `ball_stick` on an ionic solid draws the first-neighbor coordination network. | | --shading MODE | smooth | smooth or flat mesh shading | | --backend BACKEND | cpu | cpu (3D), matplotlib (projected 2D), plotly, or explicit native GPU PNG; never selected by fallback | | --orthogonal | yes | Orthographic projection | diff --git a/mat_viewer/app/backend_core.py b/mat_viewer/app/backend_core.py index 681e9cbd..cf22055a 100644 --- a/mat_viewer/app/backend_core.py +++ b/mat_viewer/app/backend_core.py @@ -6,6 +6,7 @@ from ..cache_keys import figure_state_cache_key from ..config import current_config +from ..render.radius import INORGANIC_DEFAULT_BOND_RADIUS from ..structure.bonds import inorganic_solid_profile from ..topology.analysis import _MAX_REGISTERED_CN from .camera_helpers import * @@ -33,10 +34,6 @@ # comfortably covers typical tab-switch + display-mode + slider # patterns without unbounded memory growth. FIGURE_CACHE_MAX = 16 -# Ionic and extended inorganic lattices are read more clearly with a thin -# coordination stroke. Keep the value in state construction so callers can -# still override it through a preset or an explicit state patch. -INORGANIC_DEFAULT_BOND_RADIUS = 0.12 class _CoreBackendMixin: diff --git a/mat_viewer/cli.py b/mat_viewer/cli.py index 8f196758..fa975d1c 100644 --- a/mat_viewer/cli.py +++ b/mat_viewer/cli.py @@ -1016,6 +1016,38 @@ def _render_result_payload(result, structure, args: argparse.Namespace, camera) } +def _resolve_omitted_render_style(args: argparse.Namespace) -> str: + """Pick the viewer-aligned style when ``--style`` was omitted. + + Ionic and extended inorganic lattices default to space-filling balls. + Molecules and covalent networks stay on ball-and-stick. An explicit + ``--style`` never reaches this function. + """ + from .agent import load_structure + from .render.cli_controls import bond_thresholds_from_args + from .structure.bonds import prefers_lattice_ball_default + + context = ( + redirect_stdout(sys.stderr) + if args.json_output + else redirect_stdout(sys.stdout) + ) + with context: + probe = load_structure( + args.input, + input_format=args.input_format, + type_map=args.type_map, + frame=args.frame if args.frame is not None else 0, + bond_scale=args.bond_scale, + bond_thresholds=bond_thresholds_from_args(args), + ) + bundle = probe.frames[0].bundle + scene = getattr(bundle, "scene", {}) or {} + if prefers_lattice_ball_default(scene, getattr(bundle, "raw_atoms", None)): + return "ball" + return "ball_stick" + + def _display_mode(structure, args: argparse.Namespace) -> str: if args.view != "auto": return args.view @@ -1071,6 +1103,12 @@ def _agent_render_main(args: argparse.Namespace) -> None: json_output=args.json_output, ) + if args.style is None: + try: + args.style = _resolve_omitted_render_style(args) + except Exception as exc: + _fail(str(exc), json_output=args.json_output) + try: fast_result = render_batch_if_selected( args, diff --git a/mat_viewer/render/cli.py b/mat_viewer/render/cli.py index 7e4e4368..590ed342 100644 --- a/mat_viewer/render/cli.py +++ b/mat_viewer/render/cli.py @@ -112,8 +112,12 @@ def _build_render_parser( p.add_argument( "--style", choices=_STYLES, - default="ball_stick", - help="Atom/bond rendering style (default: ball_stick).", + default=None, + help=( + "Atom/bond rendering style. Omitted: ball_stick for molecules and " + "covalent networks; ball for ionic and inorganic lattices. Explicit " + "ball_stick on an ionic solid draws lattice coordination sticks." + ), ) p.add_argument( "--shading", diff --git a/mat_viewer/render/planning.py b/mat_viewer/render/planning.py index 18dac04d..4d1fbab9 100644 --- a/mat_viewer/render/planning.py +++ b/mat_viewer/render/planning.py @@ -8,6 +8,7 @@ import numpy as np +from ..config import DEFAULT_STYLE from ..config import atom_radius as configured_atom_radius from ..config import element_color as configured_element_color from .bond_annotations import bond_annotation_primitives @@ -41,7 +42,12 @@ from .mesh_overlays import polyhedron_primitives as _polyhedron_primitives from .property_planning import prepare_render_property, property_color_for_atom, reserve_property_colorbar, resolve_render_property_context from ..style.disorder import bond_effective_opacity -from .radius import atom_visual_radius, clamp_displacement_radius, hydrogen_radius_floor +from .radius import ( + INORGANIC_DEFAULT_BOND_RADIUS, + atom_visual_radius, + clamp_displacement_radius, + hydrogen_radius_floor, +) def prepare_render( @@ -126,6 +132,17 @@ def prepare_render( else np.asarray([1.0, 1.0, 0.8]) ) + lattice_coordination = bool(scene.get("is_ionic_solid")) + stick_radius = float(render_spec.bond_radius) + if ( + lattice_coordination + and default_representation == "ball_stick" + and stick_radius == float(DEFAULT_STYLE["bond_radius"]) + ): + # The molecular stick is thick relative to an ionic display radius. + # Use the same thin coordination stroke as the viewer lattice default + # unless the caller set a different bond radius. + stick_radius = float(INORGANIC_DEFAULT_BOND_RADIUS) for index, atom in enumerate(atoms): element = ( str(_value(atom, "symbol", "elem", "element", default="C")) @@ -251,8 +268,9 @@ def prepare_render( atom, representation="ball_stick", atom_scale=render_spec.atom_scale, - bond_radius=render_spec.bond_radius, + bond_radius=stick_radius, min_atom_radius=render_spec.min_atom_radius, + lattice_coordination=lattice_coordination, ) ) displacement = np.eye(3) * (placeholder_radius / 1.54) ** 2 @@ -297,8 +315,9 @@ def prepare_render( atom, representation=representation, atom_scale=render_spec.atom_scale, - bond_radius=render_spec.bond_radius, + bond_radius=stick_radius, min_atom_radius=render_spec.min_atom_radius, + lattice_coordination=lattice_coordination, ) ) primitives.append( @@ -448,7 +467,7 @@ def prepare_render( atom_i=atoms[first_index], atom_j=atoms[second_index], ) - radius = render_spec.bond_radius * float( + radius = stick_radius * float( _value(bond, "_render_radius_scale", default=1.0) ) override_color = _value(bond, "_render_color", default=None) @@ -1272,6 +1291,7 @@ def _atom_radius( atom_scale: float = 1.0, bond_radius: float | None = None, min_atom_radius: float | None = None, + lattice_coordination: bool = False, ) -> float: value = float(_value(atom, "atom_radius", "radius", default=0.5)) rendered = atom_visual_radius( @@ -1281,6 +1301,7 @@ def _atom_radius( atom_scale=atom_scale, bond_radius=bond_radius, min_atom_radius=min_atom_radius, + lattice_coordination=lattice_coordination, ) if representation in {"space_filling", "ball"}: return max(rendered, 0.2) diff --git a/mat_viewer/render/radius.py b/mat_viewer/render/radius.py index 74956bc3..b063a8f7 100644 --- a/mat_viewer/render/radius.py +++ b/mat_viewer/render/radius.py @@ -14,6 +14,16 @@ import numpy as np HYDROGEN_RADIUS_BOND_FACTOR = 1.5 +# Thin coordination stroke for an ionic lattice ball-and-stick figure when the +# caller left the bond radius at the molecular default. The viewer state +# builder uses the same value for inorganic lattice defaults. +INORGANIC_DEFAULT_BOND_RADIUS = 0.12 +# Molecular ball-and-stick compacts spheres so the stick stays readable. +# Binary ionic lattices skip this factor and stay at the ball-style radius. +MOLECULAR_BALL_STICK_RADIUS_SCALE = 0.46 +# Ionic coordination spheres must read as balls on sticks, including elements +# whose display radius would otherwise sit inside the cylinder. +IONIC_BALL_STICK_ATOM_TO_BOND_RATIO = 1.5 def hydrogen_radius_floor( @@ -53,19 +63,35 @@ def atom_visual_radius( style: Mapping[str, Any] | None = None, bond_radius: float | None = None, min_atom_radius: float | None = None, + lattice_coordination: bool = False, ) -> float: """Return one backend-neutral rendered atom radius. Non-hydrogen caller radii are preserved. H/D receives the display floor - after the representation's compact ball-stick scale is applied. + after the representation's compact ball-stick scale is applied. A binary + ionic lattice drawn as ball-and-stick uses the ball radius, then a floor + above the stick, so coordination spheres are not swallowed. """ value = float(atom_radius) scale = float(atom_scale) if value < 0.0 or scale <= 0.0: raise ValueError("atom_radius and atom_scale must be positive") mode = str(representation).lower().replace("-", "_") - base = value if mode in {"ball", "space_filling"} else value * 0.46 + use_ball_radius = mode in {"ball", "space_filling"} or ( + lattice_coordination and mode == "ball_stick" + ) + base = value if use_ball_radius else value * MOLECULAR_BALL_STICK_RADIUS_SCALE rendered = base * scale + if ( + lattice_coordination + and mode == "ball_stick" + and bond_radius is not None + and float(bond_radius) > 0.0 + ): + rendered = max( + rendered, + IONIC_BALL_STICK_ATOM_TO_BOND_RATIO * float(bond_radius), + ) if str(element).strip().capitalize() in {"H", "D"}: rendered = max( rendered, @@ -110,6 +136,9 @@ def clamp_displacement_radius( __all__ = [ "HYDROGEN_RADIUS_BOND_FACTOR", + "INORGANIC_DEFAULT_BOND_RADIUS", + "IONIC_BALL_STICK_ATOM_TO_BOND_RATIO", + "MOLECULAR_BALL_STICK_RADIUS_SCALE", "atom_visual_radius", "clamp_displacement_radius", "hydrogen_radius_floor", diff --git a/mat_viewer/scene/core.py b/mat_viewer/scene/core.py index 3118bd9e..d631e936 100644 --- a/mat_viewer/scene/core.py +++ b/mat_viewer/scene/core.py @@ -169,11 +169,11 @@ def build_scene_from_atoms( copied.setdefault(key, copy.deepcopy(source[key])) canonical_unwrapped_atoms.append(copied) canonical_records = [dict(record) for record in molcrys_analysis.bond_records] - # MolCrysKit's signed graph intentionally includes metal--halide - # coordination contacts so topology/polyhedron analysis can reuse the - # same periodic graph. They are not covalent sticks in a binary ionic - # solid (NaCl/CsCl); retain the canonical records but suppress those - # contacts from the visible ball-and-stick bond list. + # MolCrysKit's signed graph includes metal--halide coordination contacts + # so topology and polyhedra can reuse it. For a binary ionic solid those + # contacts are the lattice ball-and-stick network: keep every lifted + # image in the visible bond list. Ball and space-filling renderers do + # not stroke scene bonds, so the unit-cell ball default stays stick-free. ionic_source_pairs = ionic_coordinate_source_pairs( source_atoms, canonical_records, @@ -346,18 +346,6 @@ def _formula_signature(items): for i, j in bond_pairs: ai = draw_atoms[i] aj = draw_atoms[j] - source_pair = tuple( - sorted( - ( - int(ai.get("_source_index", -1)), - int(aj.get("_source_index", -1)), - ) - ) - ) - if source_pair in ionic_source_pairs: - if telemetry_enabled: - ionic_bond_drops += 1 - continue if bonds_conflict(ai, aj): if telemetry_enabled: conflict_drops += 1 @@ -475,7 +463,9 @@ def _formula_signature(items): "ionic_coordinate_source_pairs": [ list(pair) for pair in sorted(ionic_source_pairs) ], - "suppressed_ionic_bond_count": len(ionic_source_pairs), + # Contacts stay in ``bonds``. The count remains for callers that + # used to observe suppression; it is zero while the sticks are visible. + "suppressed_ionic_bond_count": 0, "bond_thresholds": copy.deepcopy(bond_thresholds), "projected_axes": projected_axes, "axis_labels": axis_labels, diff --git a/mat_viewer/scene/periodic.py b/mat_viewer/scene/periodic.py index 0fcf7c74..1ea492b0 100644 --- a/mat_viewer/scene/periodic.py +++ b/mat_viewer/scene/periodic.py @@ -30,25 +30,36 @@ def scene_ops(): compute_label_positions=_compute_label_positions, ) +# Half-cell rocksalt neighbours are equidistant from the opposite image. +# Lengths that agree within this tolerance keep the manifested Cartesian +# site; a longer stored segment still snaps to the minimum image. +_BOND_ENDPOINT_TIE_TOLERANCE_A = 1e-6 + + def _bond_endpoints(ai, aj, cell, display_mode: str): + """Return the Cartesian segment for one displayed bond. + + Canonical lifting already chose the two display atoms. Their stored + coordinates are the contact when that segment is a minimum-image length. + ASE's minimum-image rounding maps an exact half-cell vector onto the + opposite image, so a rocksalt octahedron would otherwise draw one arm + twice and drop its partner. A wrapped pair whose stored carts span the + cell (for example fractional x of -0.1 and 0.6) is still lifted to the + minimum image; that shorter segment is what the long-render filter keeps. + Unwrapped molecules and cluster selections keep their stored carts. + """ start = np.array(ai["cart"], dtype=float) - if ai.get("_strict_unit_cell") or aj.get("_strict_unit_cell"): - # Strict unit-cell views wrap atom centres into the home cell. The - # source connectivity can still describe a bond whose nearest image - # crosses a cell face (for example, source coordinates at -0.1 and - # 0.6 fractional x). Connecting the wrapped centres directly would - # draw a spurious cell-spanning segment and the scene builder would - # drop it at the long-render threshold. Lift the endpoint to the - # minimum-image position while keeping both displayed atoms in the - # home cell. - return start, np.array(_nearest_pbc_cart(start, aj["cart"], cell), dtype=float) + direct = np.array(aj["cart"], dtype=float) if display_mode in ("formula_unit", "cluster") or ( ai.get("_unwrapped") and aj.get("_unwrapped") ): - end = np.array(aj["cart"], dtype=float) - else: - end = np.array(_nearest_pbc_cart(ai["cart"], aj["cart"], cell), dtype=float) - return start, end + return start, direct + minimum_image = np.array(_nearest_pbc_cart(start, aj["cart"], cell), dtype=float) + direct_len = float(np.linalg.norm(direct - start)) + minimum_image_len = float(np.linalg.norm(minimum_image - start)) + if direct_len <= minimum_image_len + _BOND_ENDPOINT_TIE_TOLERANCE_A: + return start, direct + return start, minimum_image _SOURCE_IMAGE_TOL = 1e-5 diff --git a/mat_viewer/structure/bonds.py b/mat_viewer/structure/bonds.py index a3d24e7e..3b6bbe69 100644 --- a/mat_viewer/structure/bonds.py +++ b/mat_viewer/structure/bonds.py @@ -28,27 +28,24 @@ def bonds_conflict(ai, aj): _MAX_CANDIDATE_CUTOFF = 12.0 -# A binary alkali/metal halide is represented by a coordination graph in -# MolCrysKit so that the same graph can drive periodic shells and polyhedra. -# Those contacts are not covalent bonds, however, and drawing every contact -# as a stick makes a NaCl/CsCl unit cell look like a molecule. Keep this -# small chemistry predicate here with the bond policy so scene assembly and -# transform rebuilds apply the same rule. It deliberately requires a -# *binary* composition; metal-halide bonds in a ligand complex (or in a -# covalent polyhalide) stay visible. +# A binary alkali/alkaline-earth halide is a coordination graph in MolCrysKit +# so the same graph can drive periodic shells and polyhedra. Those contacts +# are not molecular covalent bonds. Ball-and-stick still draws them, as the +# complete first-neighbor lattice network, when the caller asks for sticks. +# Keep this small chemistry predicate here with the bond policy so scene +# assembly and transform rebuilds classify the same solids. It deliberately +# requires a *binary* composition; metal-halide bonds in a ligand complex (or +# in a covalent polyhalide) stay on the ordinary covalent path. _IONIC_HALIDES = frozenset({"F", "Cl", "Br", "I"}) -# Restrict the display-only suppression to salts whose cation is normally -# presented as an ionic lattice site. Transition-metal halides and Pb--Cl -# coordination graphs can be molecular/coordination compounds; hiding those -# sticks would discard useful structural information. +# Binary salts of these cations keep lattice coordination sticks. Transition- +# metal halides and Pb--Cl graphs can be molecular compounds, so they stay on +# the ordinary covalent path. _IONIC_CATIONS = frozenset( { "Li", "Na", "K", "Rb", "Cs", "Fr", "Be", "Mg", "Ca", "Sr", "Ba", "Ra", } ) - - def inorganic_solid_profile(atoms) -> dict[str, object]: """Classify the compact inorganic-solid defaults from source atoms. @@ -98,11 +95,12 @@ def ionic_coordinate_source_pairs(atoms, bond_records) -> set[tuple[int, int]]: MolCrysKit intentionally includes metal--halide contacts in its signed ``BondRecord`` graph because the graph is also the input to packing-shell - analysis. MatterVis's ball-and-stick view should not render those - contacts as covalent rods for a binary ionic solid. The returned set is - empty for mixed ligand structures, non-binary compositions, or records - containing any non metal--halide edge, so covalent/coordination examples - keep their normal sticks. + analysis. For a binary ionic solid those contacts are the lattice + coordination network: ball-and-stick draws the full first-neighbor set, + including periodic images, instead of hiding it or treating it as a + molecular covalent graph. The returned set is empty for mixed ligand + structures, non-binary compositions, or records containing any non + metal--halide edge, so covalent examples keep their normal sticks. """ if not atoms or not bond_records: return set() @@ -137,6 +135,22 @@ def ionic_coordinate_source_pairs(atoms, bond_records) -> set[tuple[int, int]]: return pairs +def prefers_lattice_ball_default(scene=None, atoms=None) -> bool: + """Return whether an omitted render style should follow the lattice default. + + Ionic solids and extended inorganic lattices (rutile, perovskites) use + space-filling balls on the unit cell, matching the viewer. Carbon- + containing frameworks and molecules stay on molecular ball-and-stick. + Scene flags win when the loader has already classified the structure. + """ + payload = scene if isinstance(scene, Mapping) else {} + if "is_ionic_solid" in payload or "is_inorganic_solid" in payload: + return bool(payload.get("is_ionic_solid") or payload.get("is_inorganic_solid")) + if atoms is None: + return False + return bool(inorganic_solid_profile(atoms).get("is_inorganic_solid")) + + def validate_bond_scale(value: float) -> float: """Return a finite, positive bond-perception scale.""" numeric = float(value) diff --git a/mat_viewer/transforms/core.py b/mat_viewer/transforms/core.py index 1df4f214..cb801b90 100644 --- a/mat_viewer/transforms/core.py +++ b/mat_viewer/transforms/core.py @@ -764,16 +764,6 @@ def rebuild_scene_with_atoms( for i, j in bond_pairs: ai = draw_atoms[int(i)] aj = draw_atoms[int(j)] - source_pair = tuple( - sorted( - ( - int(ai.get("_source_index", -1)), - int(aj.get("_source_index", -1)), - ) - ) - ) - if source_pair in ionic_source_pairs: - continue start = np.asarray(ai["cart"], dtype=float) end = np.asarray(aj["cart"], dtype=float) bonds.append( @@ -827,7 +817,7 @@ def rebuild_scene_with_atoms( out["ionic_coordinate_source_pairs"] = [ list(pair) for pair in sorted(ionic_source_pairs) ] - out["suppressed_ionic_bond_count"] = len(ionic_source_pairs) + out["suppressed_ionic_bond_count"] = 0 return out diff --git a/skills/visualize-materials/SKILL.md b/skills/visualize-materials/SKILL.md index b1567df3..b3f2c25b 100644 --- a/skills/visualize-materials/SKILL.md +++ b/skills/visualize-materials/SKILL.md @@ -44,8 +44,8 @@ see [installation](./references/installation.md). Read [input formats](./referen ## Execution boundary -- Molecules and covalent networks default to `ball_stick`; use `ball` for dense - ionic or coordination structures only when bonds are not evidence. +- Omit `--style` on ionic/inorganic lattices for `ball` and a unit cell. Explicit + `ball_stick` there draws coordination sticks; molecules stay `ball_stick`. - If rendering starts later or is delegated, that agent loads this skill before choosing a command; parent summaries and guessed flags are not a substitute. - Render directly; do not run a separate analysis or installation preflight. diff --git a/skills/visualize-materials/references/polyhedra.md b/skills/visualize-materials/references/polyhedra.md index 3deddf90..d1310c00 100644 --- a/skills/visualize-materials/references/polyhedra.md +++ b/skills/visualize-materials/references/polyhedra.md @@ -10,8 +10,10 @@ mat-vis render INPUT -o OUTPUT.png --backend cpu --json \ --polyhedron '{"center":"Al","ligand":"O","level":"atom"}' ``` -Use `ball_stick` when discrete covalent components must retain bonds. Keep the -default center-element hue; do not name a color unless the user requested one. +Use `ball_stick` when discrete covalent components must retain bonds. On an +ionic lattice, omit `--style` for the `ball` default; explicit `ball_stick` +draws first-neighbor coordination sticks. Keep the default center-element hue; +do not name a color unless the user requested one. Omit `cutoff` for the natural shell. A supplied cutoff is a hard radial cap; `fallback_max` limits coordination count. diff --git a/skills/visualize-materials/references/quickstart.md b/skills/visualize-materials/references/quickstart.md index 6d76b56b..37048dfb 100644 --- a/skills/visualize-materials/references/quickstart.md +++ b/skills/visualize-materials/references/quickstart.md @@ -15,8 +15,11 @@ nonperiodic/synthetic-cell object with no cell. Its automatic camera faces the largest lattice face. Do not add a camera direction, replication, or cell override unless the requested evidence needs one. -Use `--style ball_stick` for molecules and covalent networks. Use `--style ball` -for a dense ionic/coordination structure when bonds are not part of the evidence. +Omit `--style` for an ionic or inorganic lattice: the CLI uses `ball` and a +unit cell. Pass `--style ball_stick` only when the figure should show the +first-neighbor coordination network (lattice sticks, not molecular covalent +sticks). Molecules and covalent networks use `--style ball_stick`. Use +`--style ball` for a dense ionic structure when bonds are not evidence. LAMMPS numeric types require the provenance-backed full `--type-map`; never guess elements from type numbers or a model filename. diff --git a/tests/render/test_ionic_coordination_sticks.py b/tests/render/test_ionic_coordination_sticks.py new file mode 100644 index 00000000..5575ab3f --- /dev/null +++ b/tests/render/test_ionic_coordination_sticks.py @@ -0,0 +1,192 @@ +"""Rock-salt ball-and-stick must show a complete octahedral coordination net.""" + +from __future__ import annotations + +import argparse +from pathlib import Path + +import numpy as np +import pytest +from ase.build import bulk +from ase.io import write + +from mat_viewer.agent import prepare_render +from mat_viewer.cli import ( + _build_render_parser, + _display_mode, + _resolve_omitted_render_style, +) +from mat_viewer.loader import build_bundle_scene +from mat_viewer.loader.structure_input import load_structure_input +from mat_viewer.structure.bonds import prefers_lattice_ball_default + +_LATTICE = 5.6402 +_HALF = _LATTICE / 2.0 + + +def _write_rocksalt(path: Path) -> None: + structure = bulk("NaCl", "rocksalt", a=_LATTICE, cubic=True) + write(path, structure, format="vasp", direct=True, vasp5=True) + + +def _loaded(path: Path): + return load_structure_input(path) + + +def _home_atoms(scene: dict) -> list[tuple[int, dict]]: + homes = [] + for index, atom in enumerate(scene["draw_atoms"]): + if atom.get("_is_framework_context_replica"): + continue + if atom.get("_is_boundary_replica") or atom.get("_is_bonded_image_replica"): + continue + homes.append((index, atom)) + return homes + + +def _neighbor_vectors(scene: dict, index: int) -> list[np.ndarray]: + vectors = [] + for bond in scene["bonds"]: + if bond["i"] == index: + vectors.append(np.asarray(bond["end"] - bond["start"], dtype=float)) + elif bond["j"] == index: + vectors.append(np.asarray(bond["start"] - bond["end"], dtype=float)) + return vectors + + +def test_rocksalt_ball_stick_has_octahedral_coordination(tmp_path: Path) -> None: + poscar = tmp_path / "POSCAR_NaCl" + _write_rocksalt(poscar) + bundle = _loaded(poscar).frames[0].bundle + scene = build_bundle_scene( + bundle, + display_mode="unit_cell", + show_hydrogen=True, + preset={}, + ) + + assert scene["is_ionic_solid"] is True + assert scene["suppressed_ionic_bond_count"] == 0 + homes = _home_atoms(scene) + assert len(homes) == 8 + assert sorted(atom["elem"] for _, atom in homes) == ["Cl", "Cl", "Cl", "Cl", "Na", "Na", "Na", "Na"] + + axis_counts = [0, 0, 0] + for bond in scene["bonds"]: + vector = np.asarray(bond["end"] - bond["start"], dtype=float) + assert float(np.linalg.norm(vector)) == pytest.approx(_HALF, abs=1e-3) + dominant = int(np.argmax(np.abs(vector))) + off_axis = [axis for axis in range(3) if axis != dominant] + assert np.all(np.abs(vector[off_axis]) < 1e-3) + axis_counts[dominant] += 1 + assert axis_counts[0] == axis_counts[1] == axis_counts[2] > 0 + + octahedron = [] + for sign in (-1.0, 1.0): + for axis in range(3): + vector = np.zeros(3) + vector[axis] = sign * _HALF + octahedron.append(vector) + for index, _atom in homes: + vectors = _neighbor_vectors(scene, index) + assert len(vectors) == 6 + for expected in octahedron: + assert any(np.allclose(vector, expected, atol=1e-3) for vector in vectors) + image_partners = 0 + for bond in scene["bonds"]: + if index not in (bond["i"], bond["j"]): + continue + other = bond["j"] if bond["i"] == index else bond["i"] + partner = scene["draw_atoms"][other] + if partner.get("_is_framework_context_replica"): + image_partners += 1 + assert image_partners == 3 + + +def test_ionic_ball_stick_spheres_are_larger_than_sticks(tmp_path: Path) -> None: + poscar = tmp_path / "POSCAR_NaCl" + _write_rocksalt(poscar) + structure = _loaded(poscar) + plan = prepare_render( + structure, + view={"display": "unit_cell", "include_boundary_replicas": True}, + render_spec={ + "representation": "ball_stick", + "backend": "cpu", + "show_cell": False, + "show_axes": False, + "width": 32, + "height": 32, + "scale": 1, + "sphere_detail": (4, 6), + "cylinder_sides": 4, + }, + ) + atom_radii = [] + bond_radii = [] + for primitive in plan.primitives: + metadata = primitive.metadata or {} + radius = metadata.get("_raster_radius") + if metadata.get("kind") == "atom" and radius is not None: + atom_radii.append(float(radius)) + if metadata.get("kind") == "bond" and radius is not None: + bond_radii.append(float(radius)) + assert atom_radii + assert bond_radii + assert min(atom_radii) > max(bond_radii) + + +def test_ionic_ball_style_does_not_stroke_coordination_contacts(tmp_path: Path) -> None: + poscar = tmp_path / "POSCAR_NaCl" + _write_rocksalt(poscar) + plan = prepare_render( + _loaded(poscar), + view={"display": "unit_cell"}, + render_spec={ + "representation": "ball", + "backend": "cpu", + "show_cell": False, + "show_axes": False, + "width": 32, + "height": 32, + "scale": 1, + "sphere_detail": (4, 6), + }, + ) + assert plan.primitives + assert all((primitive.metadata or {}).get("kind") != "bond" for primitive in plan.primitives) + + +def test_omitted_style_uses_ball_and_unit_cell_for_rocksalt(tmp_path: Path) -> None: + poscar = tmp_path / "POSCAR_NaCl" + _write_rocksalt(poscar) + args = argparse.Namespace( + input=str(poscar), + input_format=None, + type_map=None, + frame=0, + bond_scale=None, + bond_threshold_specs=(), + json_output=True, + view="auto", + show_unit_cell=None, + ) + structure = _loaded(poscar) + assert _resolve_omitted_render_style(args) == "ball" + assert _display_mode(structure, args) == "unit_cell" + + +def test_explicit_ball_stick_stays_available_and_molecules_keep_their_default() -> None: + parser = argparse.ArgumentParser() + render = _build_render_parser(parser.add_subparsers()) + omitted = render.parse_args(["nacl.poscar", "-o", "out.png"]) + explicit = render.parse_args( + ["nacl.poscar", "-o", "out.png", "--style", "ball_stick"] + ) + assert omitted.style is None + assert explicit.style == "ball_stick" + assert prefers_lattice_ball_default( + {"is_ionic_solid": False, "is_inorganic_solid": False}, + atoms=[{"elem": "C"}, {"elem": "O"}, {"elem": "H"}], + ) is False + assert prefers_lattice_ball_default(atoms=[{"elem": "Na"}, {"elem": "Cl"}]) is True diff --git a/tests/test_inorganic_defaults.py b/tests/test_inorganic_defaults.py index 0e2ea74c..8f148799 100644 --- a/tests/test_inorganic_defaults.py +++ b/tests/test_inorganic_defaults.py @@ -97,7 +97,7 @@ def test_extended_inorganic_profile_covers_oxide_lattices_without_mof_false_posi assert inorganic_solid_profile(hkust)["is_inorganic_solid"] is False -def test_nacl_keeps_mck_contact_but_hides_ionic_sticks(): +def test_nacl_keeps_mck_contacts_as_visible_coordination_sticks(): parsed = load_cif("benchmarks/structures/ionic_nacl.cif") bundle = build_loaded_crystal_from_atoms( name="ionic-nacl", @@ -109,7 +109,8 @@ def test_nacl_keeps_mck_contact_but_hides_ionic_sticks(): ) assert bundle.scene["is_ionic_solid"] is True assert bundle.scene["_canonical_bond_records"] - assert bundle.scene["bonds"] == [] + assert bundle.scene["bonds"] + assert bundle.scene["suppressed_ionic_bond_count"] == 0 unit_cell = build_bundle_scene( bundle, @@ -118,15 +119,19 @@ def test_nacl_keeps_mck_contact_but_hides_ionic_sticks(): preset={}, ) # Origin Na is a corner site (8 images) and Cl at (0.5, 0, 0) is an - # edge site (4 images). Symmetric 0/1 copies stay inside the cell; - # the old spanning-bond shell (4 atoms, including Cl at -0.5) does not. + # edge site (4 images). Symmetric 0/1 copies stay inside the cell. + # The contact graph stays visible; ball rendering does not stroke it. assert len(unit_cell["draw_atoms"]) == 12 assert unit_cell["framework_context_replica_count"] == 0 - assert unit_cell["bonds"] == [] - assert unit_cell["suppressed_ionic_bond_count"] > 0 - for atom in unit_cell["draw_atoms"]: + assert unit_cell["suppressed_ionic_bond_count"] == 0 + atoms = unit_cell["draw_atoms"] + for atom in atoms: frac = atom["frac"] assert all(-1e-8 <= float(component) <= 1.0 + 1e-8 for component in frac) + assert unit_cell["bonds"] + for bond in unit_cell["bonds"]: + pair = {atoms[bond["i"]]["elem"], atoms[bond["j"]]["elem"]} + assert pair == {"Na", "Cl"} def test_binary_ionic_default_state_uses_cell_and_atom_coordination(tmp_path: Path): From 0ba94f2cd27504eae5fc59234ef9f4ee13474b08 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 26 Oct 2026 00:34:27 +0000 Subject: [PATCH 3/4] Keep module size limits and count supercell Na-Cl junctions Move omitted-style resolution next to the other render CLI controls. A 2x2x2 repeat of the one-dimensional NaCl contact includes the junction bonds between repeated cells. Co-authored-by: Ming-Yu Guo --- mat_viewer/cli.py | 42 ++----------------- mat_viewer/render/cli.py | 6 +-- mat_viewer/render/cli_controls.py | 34 +++++++++++++++ mat_viewer/render/planning.py | 23 +++------- mat_viewer/render/radius.py | 25 +++++++++++ tests/app/test_qa_canvas_regressions.py | 16 ++++++- .../render/test_ionic_coordination_sticks.py | 9 ++-- tests/test_module_organization.py | 6 ++- 8 files changed, 92 insertions(+), 69 deletions(-) diff --git a/mat_viewer/cli.py b/mat_viewer/cli.py index fa975d1c..2a7d10b9 100644 --- a/mat_viewer/cli.py +++ b/mat_viewer/cli.py @@ -1016,38 +1016,6 @@ def _render_result_payload(result, structure, args: argparse.Namespace, camera) } -def _resolve_omitted_render_style(args: argparse.Namespace) -> str: - """Pick the viewer-aligned style when ``--style`` was omitted. - - Ionic and extended inorganic lattices default to space-filling balls. - Molecules and covalent networks stay on ball-and-stick. An explicit - ``--style`` never reaches this function. - """ - from .agent import load_structure - from .render.cli_controls import bond_thresholds_from_args - from .structure.bonds import prefers_lattice_ball_default - - context = ( - redirect_stdout(sys.stderr) - if args.json_output - else redirect_stdout(sys.stdout) - ) - with context: - probe = load_structure( - args.input, - input_format=args.input_format, - type_map=args.type_map, - frame=args.frame if args.frame is not None else 0, - bond_scale=args.bond_scale, - bond_thresholds=bond_thresholds_from_args(args), - ) - bundle = probe.frames[0].bundle - scene = getattr(bundle, "scene", {}) or {} - if prefers_lattice_ball_default(scene, getattr(bundle, "raw_atoms", None)): - return "ball" - return "ball_stick" - - def _display_mode(structure, args: argparse.Namespace) -> str: if args.view != "auto": return args.view @@ -1103,13 +1071,11 @@ def _agent_render_main(args: argparse.Namespace) -> None: json_output=args.json_output, ) - if args.style is None: - try: - args.style = _resolve_omitted_render_style(args) - except Exception as exc: - _fail(str(exc), json_output=args.json_output) - try: + if args.style is None: + from .render.cli_controls import resolve_omitted_render_style + + args.style = resolve_omitted_render_style(args) fast_result = render_batch_if_selected( args, install_command=check_payload["requirements"]["install"], diff --git a/mat_viewer/render/cli.py b/mat_viewer/render/cli.py index 590ed342..1259e18f 100644 --- a/mat_viewer/render/cli.py +++ b/mat_viewer/render/cli.py @@ -113,11 +113,7 @@ def _build_render_parser( "--style", choices=_STYLES, default=None, - help=( - "Atom/bond rendering style. Omitted: ball_stick for molecules and " - "covalent networks; ball for ionic and inorganic lattices. Explicit " - "ball_stick on an ionic solid draws lattice coordination sticks." - ), + help="Omitted: ball_stick, or ball for ionic and inorganic lattices.", ) p.add_argument( "--shading", diff --git a/mat_viewer/render/cli_controls.py b/mat_viewer/render/cli_controls.py index 240a8244..7969fac6 100644 --- a/mat_viewer/render/cli_controls.py +++ b/mat_viewer/render/cli_controls.py @@ -29,6 +29,40 @@ def add_bond_policy_arguments(parser: argparse.ArgumentParser) -> None: ) +def resolve_omitted_render_style(args: argparse.Namespace) -> str: + """Pick the viewer-aligned style when ``--style`` was omitted. + + Ionic and extended inorganic lattices default to space-filling balls. + Molecules and covalent networks stay on ball-and-stick. An explicit + ``--style`` never reaches this function. + """ + import sys + from contextlib import redirect_stdout + + from ..agent import load_structure + from ..structure.bonds import prefers_lattice_ball_default + + context = ( + redirect_stdout(sys.stderr) + if getattr(args, "json_output", False) + else redirect_stdout(sys.stdout) + ) + with context: + probe = load_structure( + args.input, + input_format=args.input_format, + type_map=args.type_map, + frame=args.frame if args.frame is not None else 0, + bond_scale=args.bond_scale, + bond_thresholds=bond_thresholds_from_args(args), + ) + bundle = probe.frames[0].bundle + scene = getattr(bundle, "scene", {}) or {} + if prefers_lattice_ball_default(scene, getattr(bundle, "raw_atoms", None)): + return "ball" + return "ball_stick" + + def bond_thresholds_from_args(args: argparse.Namespace) -> dict[tuple[str, str], float]: from ..structure.bonds import parse_bond_threshold_spec diff --git a/mat_viewer/render/planning.py b/mat_viewer/render/planning.py index 4d1fbab9..74c6a0a8 100644 --- a/mat_viewer/render/planning.py +++ b/mat_viewer/render/planning.py @@ -8,7 +8,6 @@ import numpy as np -from ..config import DEFAULT_STYLE from ..config import atom_radius as configured_atom_radius from ..config import element_color as configured_element_color from .bond_annotations import bond_annotation_primitives @@ -42,12 +41,7 @@ from .mesh_overlays import polyhedron_primitives as _polyhedron_primitives from .property_planning import prepare_render_property, property_color_for_atom, reserve_property_colorbar, resolve_render_property_context from ..style.disorder import bond_effective_opacity -from .radius import ( - INORGANIC_DEFAULT_BOND_RADIUS, - atom_visual_radius, - clamp_displacement_radius, - hydrogen_radius_floor, -) +from .radius import atom_visual_radius, clamp_displacement_radius, coordination_stick_radius, hydrogen_radius_floor def prepare_render( @@ -133,16 +127,11 @@ def prepare_render( ) lattice_coordination = bool(scene.get("is_ionic_solid")) - stick_radius = float(render_spec.bond_radius) - if ( - lattice_coordination - and default_representation == "ball_stick" - and stick_radius == float(DEFAULT_STYLE["bond_radius"]) - ): - # The molecular stick is thick relative to an ionic display radius. - # Use the same thin coordination stroke as the viewer lattice default - # unless the caller set a different bond radius. - stick_radius = float(INORGANIC_DEFAULT_BOND_RADIUS) + stick_radius = coordination_stick_radius( + render_spec.bond_radius, + representation=default_representation, + ionic_solid=lattice_coordination, + ) for index, atom in enumerate(atoms): element = ( str(_value(atom, "symbol", "elem", "element", default="C")) diff --git a/mat_viewer/render/radius.py b/mat_viewer/render/radius.py index b063a8f7..950c0c80 100644 --- a/mat_viewer/render/radius.py +++ b/mat_viewer/render/radius.py @@ -13,6 +13,8 @@ import numpy as np +from ..config import DEFAULT_STYLE + HYDROGEN_RADIUS_BOND_FACTOR = 1.5 # Thin coordination stroke for an ionic lattice ball-and-stick figure when the # caller left the bond radius at the molecular default. The viewer state @@ -26,6 +28,28 @@ IONIC_BALL_STICK_ATOM_TO_BOND_RATIO = 1.5 +def coordination_stick_radius( + bond_radius: float, + *, + representation: str, + ionic_solid: bool, +) -> float: + """Return the cylinder radius for one representation. + + Binary ionic ball-and-stick uses the viewer lattice stroke when the + caller left the molecular default in place. An explicit radius is kept. + """ + radius = float(bond_radius) + mode = str(representation).lower().replace("-", "_") + if ( + ionic_solid + and mode == "ball_stick" + and radius == float(DEFAULT_STYLE["bond_radius"]) + ): + return float(INORGANIC_DEFAULT_BOND_RADIUS) + return radius + + def hydrogen_radius_floor( style: Mapping[str, Any] | None = None, *, @@ -137,6 +161,7 @@ def clamp_displacement_radius( __all__ = [ "HYDROGEN_RADIUS_BOND_FACTOR", "INORGANIC_DEFAULT_BOND_RADIUS", + "coordination_stick_radius", "IONIC_BALL_STICK_ATOM_TO_BOND_RATIO", "MOLECULAR_BALL_STICK_RADIUS_SCALE", "atom_visual_radius", diff --git a/tests/app/test_qa_canvas_regressions.py b/tests/app/test_qa_canvas_regressions.py index 4a7c2a97..8e9da864 100644 --- a/tests/app/test_qa_canvas_regressions.py +++ b/tests/app/test_qa_canvas_regressions.py @@ -288,7 +288,21 @@ def test_repeat_counts_differ_and_clear_removes_them(tmp_path: Path): ] assert len(two_atoms) == len(home) * 8 assert len(set(identities)) == len(identities) - assert len(two_scene.get("bonds") or []) == len(home_bonds) * 8 + # This CIF is a one-dimensional Na–Cl contact. Four chains in a + # 2×2×2 repeat each keep two in-cell contacts plus the junction + # between the repeated cells. + repeated_bonds = list(two_scene.get("bonds") or []) + assert len(home_bonds) == 1 + assert len(repeated_bonds) == 4 * 3 + import numpy as np + + for bond in repeated_bonds: + pair = {two_atoms[bond["i"]]["elem"], two_atoms[bond["j"]]["elem"]} + assert pair == {"Na", "Cl"} + length = float( + np.linalg.norm(np.asarray(bond["end"]) - np.asarray(bond["start"])) + ) + assert length == pytest.approx(2.82, abs=1e-2) fig, _ = backend.figure_for_state(backend.get_state(scene_id)) box = next(trace for trace in fig.data if getattr(trace, "name", "") == "unit-cell-box") # The outline is the supercell, not the home cell the repeats grew out of. diff --git a/tests/render/test_ionic_coordination_sticks.py b/tests/render/test_ionic_coordination_sticks.py index 5575ab3f..35dd4dc3 100644 --- a/tests/render/test_ionic_coordination_sticks.py +++ b/tests/render/test_ionic_coordination_sticks.py @@ -11,11 +11,8 @@ from ase.io import write from mat_viewer.agent import prepare_render -from mat_viewer.cli import ( - _build_render_parser, - _display_mode, - _resolve_omitted_render_style, -) +from mat_viewer.cli import _build_render_parser, _display_mode +from mat_viewer.render.cli_controls import resolve_omitted_render_style from mat_viewer.loader import build_bundle_scene from mat_viewer.loader.structure_input import load_structure_input from mat_viewer.structure.bonds import prefers_lattice_ball_default @@ -172,7 +169,7 @@ def test_omitted_style_uses_ball_and_unit_cell_for_rocksalt(tmp_path: Path) -> N show_unit_cell=None, ) structure = _loaded(poscar) - assert _resolve_omitted_render_style(args) == "ball" + assert resolve_omitted_render_style(args) == "ball" assert _display_mode(structure, args) == "unit_cell" diff --git a/tests/test_module_organization.py b/tests/test_module_organization.py index 6211a5c0..1201eefb 100644 --- a/tests/test_module_organization.py +++ b/tests/test_module_organization.py @@ -16,12 +16,14 @@ "app/backend_core.py": 1518, "app/factory.py": 1116, "loader/core.py": 1224, - "cli.py": 1264, + # Omitted ``--style`` is resolved before batch selection. + "cli.py": 1268, "ortep/core.py": 1146, # Existing CLI parser is just over the hard cap while its argument # groups are being split into the dedicated controls module. "render/cli.py": 1050, - "render/planning.py": 1348, + # Ionic ball-and-stick passes the lattice stick radius into atom radii. + "render/planning.py": 1358, "structure/loader/core.py": 1050, } ALLOWED_TOP_LEVEL_PY = { From a712c652fc5cb22bd495ec9369d0ad1adcb1447c Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 26 Oct 2026 01:21:30 +0000 Subject: [PATCH 4/4] Draw rocksalt sticks on the wrapped, symmetric unit cell Lattice-site centres stay inside the closed cell, including the 0/1 boundary images. First neighbors that have no sphere there are sticks ending at the image coordinate, so each home site keeps six octahedral arms and the exterior ends are inversion-symmetric. Co-authored-by: Ming-Yu Guo --- docs/agents/scene_api.md | 12 +- mat_viewer/scene/core.py | 15 ++- mat_viewer/scene/periodic.py | 114 +++++++++++++++++- tests/app/test_qa_canvas_regressions.py | 17 +-- .../render/test_ionic_coordination_sticks.py | 70 ++++++++--- tests/render/test_nacl_ball_unit_cell.py | 11 +- 6 files changed, 207 insertions(+), 32 deletions(-) diff --git a/docs/agents/scene_api.md b/docs/agents/scene_api.md index 30865f08..2181dbaf 100644 --- a/docs/agents/scene_api.md +++ b/docs/agents/scene_api.md @@ -116,11 +116,13 @@ Parses a CIF and returns a scene dict consumable by `include_cross_boundary_bond_endpoints=False`. It is display-only. Source fractional coordinates on the loaded bundle stay unchanged. Binary alkali and alkaline-earth halide contacts stay in `scene["bonds"]`. - An explicit ball-and-stick style draws that complete first-neighbor - coordination network, including the periodic images required for the - lattice symmetry. Ball and space-filling renderers do not stroke those - contacts. When `--style` is omitted, an ionic or inorganic lattice uses - `ball` and `unit_cell`, the same default as the viewer. + An explicit ball-and-stick style draws the complete first-neighbor + network on those wrapped sites. A neighbor that has no sphere inside + the closed cell is a stick ending at that image coordinate; those ends + occur on every symmetry-equivalent face. Ball and space-filling + renderers do not stroke the contacts. When `--style` is omitted, an + ionic or inorganic lattice uses `ball` and `unit_cell`, the same + default as the viewer. - `asymmetric_unit` — only the asymmetric unit is drawn. - `cluster` — **free molecular cluster mode**. Every parsed atom is drawn unchanged; no formula-unit selection or periodic image diff --git a/mat_viewer/scene/core.py b/mat_viewer/scene/core.py index d631e936..58887851 100644 --- a/mat_viewer/scene/core.py +++ b/mat_viewer/scene/core.py @@ -76,8 +76,10 @@ ] from .periodic import ( + MAX_RENDERED_BOND_LENGTH_A, _bond_endpoints, _canonical_display_bond_pairs, + _lattice_site_image_bonds, _manifest_spanning_bond_context, _manifest_strict_bonded_images, _periodic_component_ranks, @@ -356,7 +358,7 @@ def _formula_signature(items): # and the other near the opposite face. The bond is real but its # visual representation would span the entire cell as a long line. rendered_len = float(np.linalg.norm(end - start)) - if rendered_len > 3.5: + if rendered_len > MAX_RENDERED_BOND_LENGTH_A: if telemetry_enabled: render_length_drops += 1 continue @@ -380,6 +382,17 @@ def _formula_signature(items): "depth_t": float((ai["_depth_t"] + aj["_depth_t"]) / 2.0), } ) + if lattice_site_display and display_mode == "unit_cell": + # Spheres stay on the wrapped sites. Sticks to neighbors outside + # the closed cell still complete each octahedron. + bonds.extend( + _lattice_site_image_bonds( + draw_atoms, + source_atoms, + canonical_records, + M, + ) + ) if os.environ.get("MATTERVIS_SCENE_PERF_EVENTS") == "1": perf_log.record( "scene:bonds", diff --git a/mat_viewer/scene/periodic.py b/mat_viewer/scene/periodic.py index 1ea492b0..677ae9e4 100644 --- a/mat_viewer/scene/periodic.py +++ b/mat_viewer/scene/periodic.py @@ -9,6 +9,7 @@ from ..structure.bonds import bonds_conflict from ..structure.cif_parse import parse_asu +from ..style.disorder import bond_is_disordered, bond_is_minor from ..structure.formula_unit import select_formula_unit from ..structure.geometry import _nearest_pbc_cart, view_rotation from ..style.disorder import disorder_alpha, is_minor @@ -34,6 +35,9 @@ def scene_ops(): # Lengths that agree within this tolerance keep the manifested Cartesian # site; a longer stored segment still snaps to the minimum image. _BOND_ENDPOINT_TIE_TOLERANCE_A = 1e-6 +# Cross-cell segments longer than a nearest-neighbor contact are not drawn. +# The same cutoff drops wrapped pairs whose stored carts span the cell. +MAX_RENDERED_BOND_LENGTH_A = 3.5 def _bond_endpoints(ai, aj, cell, display_mode: str): @@ -61,6 +65,114 @@ def _bond_endpoints(ai, aj, cell, display_mode: str): return start, direct return start, minimum_image + +def _lattice_site_image_bonds( + draw_atoms: list[dict[str, Any]], + source_atoms: list[dict[str, Any]], + canonical_bond_records: list[dict[str, Any]], + M: Any, +) -> list[dict[str, Any]]: + """Sticks from in-cell sites to first neighbors that have no sphere. + + Lattice-site display keeps every centre in the closed cell and only + copies the symmetric 0/1 boundary images. A rocksalt corner still has + three neighbors outside that set. The stick ends on the image + coordinate and takes its far colour from the wrapped partner, so each + home site keeps six octahedral arms without a one-sided atom shell. + """ + instances: dict[tuple[int, tuple[int, int, int]], int] = {} + partner_index: dict[int, int] = {} + for draw_index, atom in enumerate(draw_atoms): + identity = source_image_identity(atom, source_atoms, draw_index) + if identity is None: + continue + instances.setdefault(identity, draw_index) + source, shift = identity + if source not in partner_index or shift == (0, 0, 0): + partner_index[source] = instances[identity] + + neighbors: dict[int, list[tuple[int, tuple[int, int, int]]]] = {} + for record in canonical_bond_records: + try: + left = int(record["left"]) + right = int(record["right"]) + relation = tuple(int(value) for value in record["right_image_shift"]) + except (KeyError, TypeError, ValueError): + continue + if len(relation) != 3: + continue + neighbors.setdefault(left, []).append((right, relation)) + neighbors.setdefault(right, []).append( + (left, tuple(-value for value in relation)) + ) + + M_arr = np.asarray(M, dtype=float) + bonds: list[dict[str, Any]] = [] + seen: set[tuple[tuple[float, ...], tuple[float, ...]]] = set() + for draw_index, atom in enumerate(draw_atoms): + identity = source_image_identity(atom, source_atoms, draw_index) + if identity is None: + continue + source, image = identity + start = np.asarray(atom["cart"], dtype=float) + for target, shift in neighbors.get(source, ()): + target_image = tuple(image[axis] + shift[axis] for axis in range(3)) + if (target, target_image) in instances: + continue + partner = partner_index.get(target) + if partner is None or not 0 <= target < len(source_atoms): + continue + base = np.asarray( + source_atoms[target].get( + "_wrapped_frac", source_atoms[target].get("frac") + ), + dtype=float, + ) + if base.shape != (3,) or not np.all(np.isfinite(base)): + continue + wrapped = np.mod(base, 1.0) + wrapped[np.isclose(wrapped, 1.0, rtol=0.0, atol=1e-9)] = 0.0 + end = np.asarray( + frac_to_cart(wrapped + np.asarray(target_image, dtype=float), M_arr), + dtype=float, + ) + if float(np.linalg.norm(end - start)) > MAX_RENDERED_BOND_LENGTH_A: + continue + key = tuple( + sorted( + ( + tuple(np.round(start, 5)), + tuple(np.round(end, 5)), + ) + ) + ) + if key in seen or key[0] == key[1]: + continue + seen.add(key) + other = draw_atoms[partner] + bonds.append( + { + "i": draw_index, + "j": partner, + "start": start.copy(), + "end": end.copy(), + "color_i": atom["color"], + "color_j": other["color"], + "alpha_i": atom["disorder_alpha"], + "alpha_j": other["disorder_alpha"], + "is_minor": bond_is_minor(atom, other), + "is_disordered": bond_is_disordered(atom, other), + "disorder_resolved": bool( + atom.get("disorder_resolved", False) + and other.get("disorder_resolved", False) + ), + "occ": min(float(atom.get("occ", 1.0)), float(other.get("occ", 1.0))), + "depth_t": float((atom["_depth_t"] + other["_depth_t"]) / 2.0), + } + ) + return bonds + + _SOURCE_IMAGE_TOL = 1e-5 def _periodic_component_ranks( @@ -228,7 +340,7 @@ def _add_image(source: int, shift: tuple[int, int, int]) -> None: np.asarray(candidate["cart"]) - np.asarray(source_atom["cart"]) ) ) - > 3.5 + > MAX_RENDERED_BOND_LENGTH_A ): continue _add_image(target, shift) diff --git a/tests/app/test_qa_canvas_regressions.py b/tests/app/test_qa_canvas_regressions.py index 8e9da864..6f7ded54 100644 --- a/tests/app/test_qa_canvas_regressions.py +++ b/tests/app/test_qa_canvas_regressions.py @@ -268,14 +268,19 @@ def test_repeat_counts_differ_and_clear_removes_them(tmp_path: Path): base_scene = backend.scene_for_state(backend.get_state(scene_id)) base_atoms = list(base_scene.get("draw_atoms") or []) home = _home_atoms(base_atoms) - home_ids = {id(atom) for atom in home} - home_index = { - index for index, atom in enumerate(base_atoms) if id(atom) in home_ids - } + import numpy as np + + home_carts = [np.asarray(atom["cart"], dtype=float) for atom in home] + + def _on_home(point) -> bool: + return any(np.allclose(point, cart, atol=1e-3) for cart in home_carts) + + # Image sticks borrow the wrapped partner index, so count a home + # contact only when both geometric ends sit on home centres. home_bonds = [ bond for bond in (base_scene.get("bonds") or []) - if bond["i"] in home_index and bond["j"] in home_index + if _on_home(bond["start"]) and _on_home(bond["end"]) ] backend.add_transform("repeat", {"a": 2, "b": 2, "c": 2}, scene_id=scene_id) two_scene = backend.scene_for_state(backend.get_state(scene_id)) @@ -294,8 +299,6 @@ def test_repeat_counts_differ_and_clear_removes_them(tmp_path: Path): repeated_bonds = list(two_scene.get("bonds") or []) assert len(home_bonds) == 1 assert len(repeated_bonds) == 4 * 3 - import numpy as np - for bond in repeated_bonds: pair = {two_atoms[bond["i"]]["elem"], two_atoms[bond["j"]]["elem"]} assert pair == {"Na", "Cl"} diff --git a/tests/render/test_ionic_coordination_sticks.py b/tests/render/test_ionic_coordination_sticks.py index 35dd4dc3..5eabda0e 100644 --- a/tests/render/test_ionic_coordination_sticks.py +++ b/tests/render/test_ionic_coordination_sticks.py @@ -1,8 +1,14 @@ -"""Rock-salt ball-and-stick must show a complete octahedral coordination net.""" +"""Rock-salt ball-and-stick must show a complete octahedral coordination net. + +Atom centres stay on the wrapped crystallographic sites and their symmetric +0/1 boundary images. Sticks to neighbors outside that set end at the image +coordinate, on every symmetry-equivalent face. +""" from __future__ import annotations import argparse +from itertools import permutations from pathlib import Path import numpy as np @@ -42,19 +48,43 @@ def _home_atoms(scene: dict) -> list[tuple[int, dict]]: def _neighbor_vectors(scene: dict, index: int) -> list[np.ndarray]: + """Incident sticks by geometry. + + An image stick borrows its far colour from the wrapped partner, so the + partner index is not the geometric end. + """ + origin = np.asarray(scene["draw_atoms"][index]["cart"], dtype=float) vectors = [] for bond in scene["bonds"]: - if bond["i"] == index: - vectors.append(np.asarray(bond["end"] - bond["start"], dtype=float)) - elif bond["j"] == index: - vectors.append(np.asarray(bond["start"] - bond["end"], dtype=float)) + start = np.asarray(bond["start"], dtype=float) + end = np.asarray(bond["end"], dtype=float) + if np.allclose(start, origin, atol=1e-3): + vectors.append(end - start) + elif np.allclose(end, origin, atol=1e-3): + vectors.append(start - end) return vectors +def _frac_key(frac) -> tuple[float, float, float]: + rounded = np.round(np.asarray(frac, dtype=float), 5) + rounded[np.abs(rounded) < 1e-8] = 0.0 + return tuple(float(value) for value in rounded) + + +def _assert_inversion_and_cubic(points: list[tuple[float, float, float]]) -> None: + present = set(points) + for frac in points: + inverted = _frac_key(tuple(1.0 - value for value in frac)) + assert inverted in present + for order in permutations(range(3)): + assert _frac_key(tuple(frac[axis] for axis in order)) in present + + def test_rocksalt_ball_stick_has_octahedral_coordination(tmp_path: Path) -> None: poscar = tmp_path / "POSCAR_NaCl" _write_rocksalt(poscar) - bundle = _loaded(poscar).frames[0].bundle + loaded = _loaded(poscar) + bundle = loaded.frames[0].bundle scene = build_bundle_scene( bundle, display_mode="unit_cell", @@ -64,11 +94,27 @@ def test_rocksalt_ball_stick_has_octahedral_coordination(tmp_path: Path) -> None assert scene["is_ionic_solid"] is True assert scene["suppressed_ionic_bond_count"] == 0 + assert scene["framework_context_replica_count"] == 0 + assert scene["bonded_image_replica_count"] == 0 homes = _home_atoms(scene) assert len(homes) == 8 assert sorted(atom["elem"] for _, atom in homes) == ["Cl", "Cl", "Cl", "Cl", "Na", "Na", "Na", "Na"] + assert not any( + atom.get("_is_framework_context_replica") or atom.get("_is_bonded_image_replica") + for atom in scene["draw_atoms"] + ) + + matrix = np.asarray(bundle.M, dtype=float) + inverse = np.linalg.inv(matrix) + centre_fracs = [ + _frac_key(np.asarray(atom["frac"], dtype=float)) for atom in scene["draw_atoms"] + ] + for frac in centre_fracs: + assert all(-1e-8 <= component <= 1.0 + 1e-8 for component in frac) + _assert_inversion_and_cubic(centre_fracs) axis_counts = [0, 0, 0] + endpoint_fracs = [] for bond in scene["bonds"]: vector = np.asarray(bond["end"] - bond["start"], dtype=float) assert float(np.linalg.norm(vector)) == pytest.approx(_HALF, abs=1e-3) @@ -76,7 +122,10 @@ def test_rocksalt_ball_stick_has_octahedral_coordination(tmp_path: Path) -> None off_axis = [axis for axis in range(3) if axis != dominant] assert np.all(np.abs(vector[off_axis]) < 1e-3) axis_counts[dominant] += 1 + endpoint_fracs.append(_frac_key(np.asarray(bond["start"], dtype=float) @ inverse)) + endpoint_fracs.append(_frac_key(np.asarray(bond["end"], dtype=float) @ inverse)) assert axis_counts[0] == axis_counts[1] == axis_counts[2] > 0 + _assert_inversion_and_cubic(endpoint_fracs) octahedron = [] for sign in (-1.0, 1.0): @@ -89,15 +138,6 @@ def test_rocksalt_ball_stick_has_octahedral_coordination(tmp_path: Path) -> None assert len(vectors) == 6 for expected in octahedron: assert any(np.allclose(vector, expected, atol=1e-3) for vector in vectors) - image_partners = 0 - for bond in scene["bonds"]: - if index not in (bond["i"], bond["j"]): - continue - other = bond["j"] if bond["i"] == index else bond["i"] - partner = scene["draw_atoms"][other] - if partner.get("_is_framework_context_replica"): - image_partners += 1 - assert image_partners == 3 def test_ionic_ball_stick_spheres_are_larger_than_sticks(tmp_path: Path) -> None: diff --git a/tests/render/test_nacl_ball_unit_cell.py b/tests/render/test_nacl_ball_unit_cell.py index 56540acd..07c8895e 100644 --- a/tests/render/test_nacl_ball_unit_cell.py +++ b/tests/render/test_nacl_ball_unit_cell.py @@ -132,7 +132,8 @@ def test_ball_unit_cell_has_symmetric_boundary_images_only(tmp_path: Path) -> No assert scene["display_mode"] == "unit_cell" assert scene["is_ionic_solid"] is True assert scene["is_inorganic_solid"] is True - assert scene["bonds"] == [] + assert scene["suppressed_ionic_bond_count"] == 0 + assert scene["bonds"] assert scene["framework_context_replica_count"] == 0 assert scene["bonded_image_replica_count"] == 0 assert not any( @@ -161,6 +162,9 @@ def test_ball_unit_cell_has_symmetric_boundary_images_only(tmp_path: Path) -> No shading="flat", ), ) + assert all( + (primitive.metadata or {}).get("kind") != "bond" for primitive in plan.primitives + ) matrix = np.asarray(loaded.frames[0].bundle.M, dtype=float) centres = [ np.asarray(primitive.metadata["_raster_center"], dtype=float) @@ -221,7 +225,8 @@ def test_ball_unit_cell_without_replicas_stays_on_wrapped_sites(tmp_path: Path) keys = _draw_keys(scene) assert len(scene["draw_atoms"]) == 8 assert scene["bonded_image_replica_count"] == 0 - assert scene["bonds"] == [] + assert scene["bonds"] + assert scene["suppressed_ionic_bond_count"] == 0 _assert_inside_closed_cell(keys) assert sorted(keys) == sorted(expected) @@ -234,7 +239,7 @@ def test_ionic_formula_unit_centres_stay_in_the_home_cell(tmp_path: Path) -> Non mode="formula_unit", ) keys = _draw_keys(scene) - assert scene["bonds"] == [] + assert scene["suppressed_ionic_bond_count"] == 0 assert keys _assert_inside_closed_cell(keys) assert all(