perf!: make importing Auto3D 44x cheaper, and harden the tests that guard the coming refactors

CHANGELOG.md

@@ -50,6 +50,32 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   mass of the isotope it names; the change applies only where no isotope was
   specified.
 
+- **`import Auto3D` no longer imports torch or RDKit, and four attributes are
+  gone from the package namespace.** `Auto3D.ANI2xt`, `Auto3D.warnings`,
+  `Auto3D.version` and `Auto3D.PackageNotFoundError` were never public API —
+  they were leaked into the namespace by three eager optional-dependency probes
+  and by module-level imports. All four are removed.
+
+  The probes existed to detect whether an optional engine was installed, but
+  probing for ANI2xt reached `batch_opt`, which reached the `utils` barrel,
+  which reached `validation`, which imported torch and `models.loading`. So
+  importing the package paid for the whole package, plus torch and RDKit,
+  before the caller had asked for anything. Every real probe already exists at
+  its use site, so nothing was lost by deleting them.
+
+  | | before | after |
+  |---|---|---|
+  | `import Auto3D` | 1.35 s | 0.031 s |
+  | `len(sys.modules)` | 1175 | 154 |
+  | torch / RDKit loaded | yes | no |
+  | `Auto3D.*` submodules loaded | 20 | 0 |
+
+  **What stops working:** referencing any of those four names through the
+  `Auto3D` package object. **What to do instead:** `import warnings` yourself;
+  read the version from `Auto3D.__version__`; import optional engines from the
+  module that owns them. Documented public names are unaffected and still
+  resolve lazily on first access.
+
 - **One exit-code scheme, used by every command.** `cli/errors.py` has mapped
   exception types to differentiated exit codes since 3.x -- 0 success, 1
   generic, 2 configuration/input, 3 dependency, 4 GPU, 5 model, plus 6 for a

src/Auto3D/__init__.py

@@ -5,32 +5,45 @@
 using neural network potentials (AIMNet2, ANI2x, ANI2xt).
 """
 
-import warnings
-from importlib.metadata import PackageNotFoundError, version
 
-try:
-    __version__ = version(__name__)
-except PackageNotFoundError:
-    __version__ = "unknown"
+def _detect_version() -> str:
+    """Read the installed distribution version, or "unknown" if not installed.
 
-# Optional dependency imports with proper exception handling
-with warnings.catch_warnings():
-    warnings.simplefilter("ignore")
+    The ``importlib.metadata`` import lives inside this function on purpose: at
+    module level it would put ``version`` and ``PackageNotFoundError`` into the
+    package namespace, where they are reachable as ``Auto3D.version`` and
+    ``Auto3D.PackageNotFoundError`` -- two names this package never meant to
+    export. See ``tests/test_import_boundaries.py``.
+    """
+    from importlib.metadata import PackageNotFoundError, version
 
     try:
-        from openeye import oechem, oeomega, oequacpac  # noqa: F401  (optional dependency probe)
-    except ImportError:
-        pass  # OpenEye is optional
+        return version(__name__)
+    except PackageNotFoundError:
+        return "unknown"
 
-    try:
-        import torchani  # noqa: F401  (optional dependency probe)
-    except ImportError:
-        pass  # TorchANI is optional
 
-    try:
-        from Auto3D.batch_opt.ANI2xt_no_rep import ANI2xt  # noqa: F401  (optional dependency probe)
-    except ImportError:
-        pass  # ANI2xt model is optional
+__version__ = _detect_version()
+
+# NOTE: this module imports nothing but the standard library, and that is a
+# tested property, not an accident (tests/test_import_boundaries.py). Importing
+# the package root is the cost every consumer pays unconditionally -- `auto3d
+# --help`, a build script reading `__version__`, a tool that merely lists
+# installed distributions -- and none of them need torch or rdkit.
+#
+# This file used to end with three eager optional-dependency probes (openeye,
+# torchani, and `from Auto3D.batch_opt.ANI2xt_no_rep import ANI2xt`) wrapped in
+# `warnings.catch_warnings()`. Nothing consumed any of them, and the third one
+# defeated the `_LAZY_API` mechanism below outright: it reached ANI2xt_no_rep ->
+# the Auto3D.utils barrel -> utils.validation -> Auto3D.models.* + torch, which
+# turned `import Auto3D` into 1175 modules and 1.35 s and eagerly loaded 20
+# Auto3D submodules. Every probe is already duplicated where it is load-bearing
+# and where its result is actually used:
+#   * openeye  -> isomer_engine.py (names used at call time) and
+#                 utils/validation.py (raises DependencyError with a fix hint)
+#   * torchani -> utils/validation.py (same) and batch_opt/ANI2xt_no_rep.py
+#   * ANI2xt   -> constructed only through model_factory / models.adapter
+# Do not reintroduce a probe here. A dependency is checked where it is needed.
 
 __all__ = [
     "__version__",
@@ -73,9 +86,38 @@
 
 # Lazy imports for public API
 def __getattr__(name: str):
-    """Lazy import for public API functions (see _LAZY_API)."""
+    """Lazy import for public API functions (see _LAZY_API).
+
+    Design constraint, not a missed optimization: this **must not** cache the
+    resolved object into ``globals()``. Caching would turn every access after
+    the first into a snapshot, i.e. exactly the import-time binding that makes
+    ``from X import y`` capture a stub -- a test that touches ``Auto3D.main``
+    and then patches ``Auto3D.auto3D.main`` would patch nothing, report
+    success, and fail somewhere else entirely (the mechanism written up at
+    length in ``tests/test_lazy_torchani_import.py``). After the first access
+    ``import_module`` is a ``sys.modules`` dict lookup, so a cache buys nothing
+    measurable. Pinned by
+    ``test_import_boundaries.py::test_getattr_does_not_cache_resolved_attributes``.
+    """
     if name in _LAZY_API:
         import importlib
         module_name, attr = _LAZY_API[name]
         return getattr(importlib.import_module(module_name), attr)
     raise AttributeError(f"module {__name__!r} has no attribute {name!r}")
+
+
+def __dir__() -> list[str]:
+    """Report the public API alongside whatever is genuinely present.
+
+    PEP 562 requires this to accompany ``__getattr__``: a module-level
+    ``__getattr__`` resolves names that are not in ``globals()``, and the
+    default ``dir()`` only sees ``globals()`` -- so without this, ``"main" in
+    dir(Auto3D)`` was False and neither tab-completion nor introspection could
+    find the public API.
+
+    The union (rather than ``sorted(__all__)``) matters because ``__dir__``
+    replaces the default entirely: imported submodules become real attributes
+    of the package, so ``Auto3D.cli`` after ``import Auto3D.cli`` must stay
+    visible.
+    """
+    return sorted(set(globals()) | set(__all__))

tests/test_SPE.py

@@ -7,9 +7,13 @@
 # from tests import skip_ani2xt_test
 skip_ani2xt_test = False
 
-# Mark all tests in this module as slow (single-point energy calculations)
-pytestmark = pytest.mark.slow
-
+# Every real-model test below is marked @pytest.mark.slow individually
+# (single-point energy calculations, each loading a real NNP). NOT a
+# module-level `pytestmark`: test_calc_spe_uses_model_factory below mocks
+# every model-construction call (create_model/EnForce_ANI/pad_from_mols) and
+# loads no NNP, so it must run in the fast tier -- a module-level mark would
+# have swept it in with everything else regardless of what it actually does.
+#
 # Every calc_spe call below passes use_gpu=False on purpose. calc_spe's
 # `use_gpu` default is True, and Auto3D 4.0 made "GPU requested but no CUDA
 # device visible" FATAL rather than a silent CPU fallback
@@ -138,6 +142,7 @@ def forward(self,
         return out['energy'].reshape(-1)
 
 
+@pytest.mark.slow
 @pytest.mark.skipif(skip_ani2xt_test, reason="ANI2xt model is not  installed.")
 def test_calc_spe_ani2xt():
     #load B97-3c results file
@@ -154,6 +159,7 @@ def test_calc_spe_ani2xt():
         assert(diff <= 0.01)
 
 
+@pytest.mark.slow
 def test_calc_spe_ani2x():
     #load wB97X/6-31G* output file
     path = os.path.join(folder, "tests/files/wb97x_dz.sdf")
@@ -170,6 +176,7 @@ def test_calc_spe_ani2x():
         print(idx, spe_out, diff)
         assert(diff <= 0.011)
 
+@pytest.mark.slow
 def test_calc_spe_aimnet():
     path = os.path.join(folder, 'tests/files/cyclooctane.sdf')
     e_ref = -314.689736079491
@@ -179,6 +186,7 @@ def test_calc_spe_aimnet():
     e_out = float(mol.GetProp('E_hartree'))
     assert(abs(e_out - e_ref) <= 0.01)    
 
+@pytest.mark.slow
 @pytest.mark.skipif(not test_userNNP1, reason="TorchANI is not  installed.")
 def test_calc_spe_userNNP1():
     #load wB97X/6-31G* output file
@@ -203,6 +211,7 @@ def test_calc_spe_userNNP1():
         assert(diff <= 0.011)
 
 
+@pytest.mark.slow
 def test_calc_spe_userNNP2():
     path = os.path.join(folder, 'tests/files/cyclooctane.sdf')
     e_ref = -314.689736079491

tests/test_batchopt.py

@@ -104,8 +104,13 @@ def mock_forward(coords, species, charges, atom_mask=None):
 
         energy, forces = model.forward_batched(coords, species, charges)
 
-        # Should have called forward multiple times due to batching
-        assert mock_adapter.forward.call_count >= 1
+        # batch_size = max(1, batchsize_atoms // N) = max(1, 10 // 5) = 2
+        # molecules per sub-batch; 4 molecules split into chunks of 2 -> the
+        # adapter must be called exactly twice, not "at least once" (which a
+        # single unbatched call would also satisfy, defeating the point of a
+        # forward_batched-specific test). Mirrors test_model_wrapper.py's
+        # stronger call_count==2 sibling for the same batchsize_atoms/N ratio.
+        assert mock_adapter.forward.call_count == 2
         assert energy.shape == (4,)
         assert forces.shape == (4, 5, 3)
 
@@ -134,13 +139,19 @@ def test_ensemble_opt_returns_convergence_info(self):
 
         result = ensemble_opt(model, coord, numbers, charges, param, torch.device("cpu"))
 
-        # Verify new fields are present
+        # Verify new fields are present, with their actual VALUES: zero force
+        # on step 1 means fmax (0.0) is at once below opttol (0.01) for both
+        # structures, so both must be reported converged and neither must
+        # have been counted as oscillating -- checking only key
+        # presence/type/length (as before) would pass even if the values
+        # were transposed, all-False, or a stray increment leaked into
+        # oscillating_count.
         assert 'converged_mask' in result, "converged_mask missing from ensemble_opt return"
         assert 'oscillating_count' in result, "oscillating_count missing from ensemble_opt return"
         assert isinstance(result['converged_mask'], list)
         assert isinstance(result['oscillating_count'], list)
-        assert len(result['converged_mask']) == 2
-        assert len(result['oscillating_count']) == 2
+        assert result['converged_mask'] == [True, True]
+        assert result['oscillating_count'] == [0, 0]
 
 
 @pytest.mark.slow

tests/test_cli_config_schema.py

@@ -5,12 +5,6 @@
 from pathlib import Path
 
 
-def test_config_schema_exists():
-    """Config schema class should exist."""
-    from Auto3D.cli.config_schema import CLIConfig
-    assert CLIConfig is not None
-
-
 def test_config_defaults():
     """Config should have sensible defaults."""
     from Auto3D.cli.config_schema import CLIConfig

tests/test_cli_security.py

@@ -107,15 +107,22 @@ def test_a_whitespace_only_file_is_refused(self, tmp_path):
             load_yaml_config(cfg)
 
     def test_a_top_level_list_is_refused(self, tmp_path):
+        """``match=`` anchors on the phrase unique to THIS guard's message
+        ("...its top level is a {type}.") -- "must contain a YAML mapping" is
+        also a substring of the empty-file message (config_schema.py:341), so
+        that alone cannot tell this test apart from the empty-file guard
+        firing by mistake (e.g. if the not-a-mapping check were deleted and
+        the empty-file check's message merely happened to also match).
+        """
         cfg = _write(tmp_path, "- k\n- window\n")
 
-        with pytest.raises(ConfigurationError, match="must contain a YAML mapping"):
+        with pytest.raises(ConfigurationError, match="top level is a list"):
             load_yaml_config(cfg)
 
     def test_a_top_level_scalar_is_refused(self, tmp_path):
         cfg = _write(tmp_path, "just a bare string\n")
 
-        with pytest.raises(ConfigurationError, match="must contain a YAML mapping"):
+        with pytest.raises(ConfigurationError, match="top level is a str"):
             load_yaml_config(cfg)
 
     def test_unparseable_yaml_is_refused(self, tmp_path):

tests/test_config.py

@@ -125,34 +125,33 @@ def test_gpu_idx_list(self):
         config = Auto3DOptions(gpu_idx=[0, 1, 2])
         assert config.gpu_idx == [0, 1, 2]
 
-    def test_immutable_default_list(self):
-        """Test that default list values are not shared between instances."""
-        config1 = Auto3DOptions()
-        config2 = Auto3DOptions()
-
-        # If gpu_idx is a list, modifying one shouldn't affect the other
-        # But with default int value, this just verifies independence
-        config1["k"] = 5
-        assert config2.k is False  # Default is False, not None
-
 
 class TestChunkMeta:
     """Tests for ChunkMeta TypedDict."""
 
     def test_chunk_meta_structure(self):
-        """Test that ChunkMeta can be used as expected."""
+        """Track the TypedDict's OWN declared keys, not a hand-copied literal.
+
+        The prior version built its own 5-key dict and asserted back the
+        values it had just set -- a key added to or removed from the real
+        ChunkMeta would never move this test. A TypedDict has no runtime
+        constructor to validate against, so pin it via ``__annotations__``/
+        ``__required_keys__`` instead: a change to config.py's ChunkMeta now
+        has to be reflected here too, or this test fails.
+        """
         from Auto3D.config import ChunkMeta
 
-        meta: ChunkMeta = {
-            "output": "/path/to/output.sdf",
-            "optimized_og": "/path/to/optimized.sdf",
-            "enumerated_sdf": "/path/to/enumerated.sdf",
-            "sorted_sdf": "/path/to/sorted.sdf",
-            "housekeeping_folder": "/path/to/housekeeping",
+        expected_keys = {
+            "output", "optimized_og", "output_taut", "smiles_enumerated",
+            "smiles_reduced", "smiles_hashed", "enumerated_sdf", "sorted_sdf",
+            "housekeeping_folder", "path", "dir",
         }
-
-        assert meta["output"] == "/path/to/output.sdf"
-        assert meta["housekeeping_folder"] == "/path/to/housekeeping"
+        assert set(ChunkMeta.__annotations__) == expected_keys
+        # ChunkMeta declares no Optional/NotRequired fields, so every key is
+        # required -- a self-consistency check that would catch a field
+        # becoming optional without a matching intent.
+        assert ChunkMeta.__required_keys__ == frozenset(expected_keys)
+        assert ChunkMeta.__optional_keys__ == frozenset()
 
 
 def test_optimization_config_exposes_no_energy_criterion_knobs():
@@ -186,6 +185,42 @@ def test_capacity_default_matches_across_layers():
     assert Auto3DOptions(path="x.smi").capacity == CLIConfig(path="x.smi").capacity
 
 
+@pytest.mark.xfail(
+    strict=True,
+    reason=(
+        "config.py FIELD_BOUNDS['opt_steps'] declares a floor of ('ge', 1), "
+        "but Auto3D.utils.validation.check_valid_configuration (validation.py:621) "
+        "and check_input (validation.py:354) each hand-write a floor of 10. "
+        "A value FIELD_BOUNDS calls valid is rejected by the other validator. "
+        "This test does not decide which number is right -- only that the two "
+        "must agree on ONE minimum, whichever the eventual fix picks."
+    ),
+)
+def test_opt_steps_minimum_agrees_between_config_and_validation():
+    """FIELD_BOUNDS['opt_steps'] and utils.validation's opt_steps floor must
+    be the SAME minimum -- see the xfail reason for the defect this pins.
+    """
+    from Auto3D.config import FIELD_BOUNDS, check_field_bounds
+    from Auto3D.utils.validation import check_valid_configuration
+
+    kind, bound_min = FIELD_BOUNDS["opt_steps"]
+    assert kind == "ge"
+
+    # config.py's own gate must accept its own declared floor (premise, not
+    # the point of this test).
+    check_field_bounds({"opt_steps": bound_min})  # must not raise
+
+    # utils.validation must not disagree with config.py's declared floor:
+    # a value config.py calls valid must not be flagged as an error there.
+    errors = check_valid_configuration(
+        path=None, k=1, opt_steps=int(bound_min), use_gpu=False,
+    )
+    assert not any("opt_steps" in e for e in errors), (
+        f"config.py says opt_steps={bound_min} is valid (FIELD_BOUNDS), but "
+        f"utils.validation.check_valid_configuration disagrees: {errors}"
+    )
+
+
 def test_negative_k_rejected():
     from Auto3D.config import Auto3DOptions
     from Auto3D.exceptions import ConfigurationError