fix: handle disabled ionization and zero-ion systems

[maxscheurer]

Closes #42

Fix the zero-ion mixedbox build path and allow ionization to be explicitly disabled with null or false input.

Implementation plan posted as a comment below.

[maxscheurer]

Implementation Plan

Analysis

ionize_solvated_system already treats a None ionization configuration as a no-op, but the normal composition models require an IonizationConfig, so that path cannot currently be selected from a Python input or YAML. In addition, concentration=0.0 is valid and, when neutralize=False, produces zero sodium and chloride counts. The wrapper still calls ionize, whose final MDAnalysis.Merge receives empty cation and anion groups and raises ValueError: cannot merge empty AtomGroup.

The ionization setting is shared by the mixedbox, bilayer, and LNP composition models. BuildInput also serializes ionization through model_dump() in metadata, so explicit disabling must be safe there as well as in the build path.

Deliverables

1. Add a zero-ion no-op to ionize_solvated_system after the requested ion counts have been computed. It will log that no ions are required, return the original universe and an empty additional-species list, and avoid calling ionize. Existing counter-ion behavior must remain active whenever either requested count is nonzero.
2. Allow the ionization field on the ionization-bearing composition models to accept IonizationConfig, None, or the literal boolean False. Normalize both explicit disable forms to the existing None no-op representation, while retaining the IonizationConfig default when the field is omitted and preserving validation for unsupported values.
3. Make BuildInput metadata serialization handle a disabled ionization value without calling model_dump() on None. The canonical model/CSV round-trip representation will remain valid.
4. Document ionization: null and ionization: false as explicit opt-out forms, including their no-ion semantics, while keeping the existing configuration examples and defaults clear.

Files to create or modify

• mdfactory/build.py — add the zero-count early return in ionize_solvated_system.
• mdfactory/models/composition.py — update the shared ionization field typing/normalization for mixedbox, bilayer, and LNP inputs; keep omitted-field defaults unchanged.
• mdfactory/models/input.py — serialize enabled ionization configs and disabled values safely in metadata.
• mdfactory/tests/test_build.py — add a focused regression test for concentration=0.0 with neutralize=False, verifying no ionization call/effect and an empty additional-species list.
• mdfactory/tests/test_models.py — test direct None and False construction, BuildInput dict/type-dispatch input, default preservation, metadata serialization, and data-row round trips for disabled ionization.
• docs/content/docs/user-guide/system-types.mdx — describe the shared opt-out forms and update the field type/behavior.
• docs/content/docs/user-guide/yaml-format.mdx — document null/false YAML input and the resulting no-op behavior.

No generated API documentation files will be edited manually; hand-written documentation is the affected docs layer.

Testing approach

• Exercise the wrapper with a water-containing universe and a zero-ion IonizationConfig; assert the universe is returned unchanged, no ion species are reported, and the lower-level ionize routine is not invoked.
• Exercise both Python-level ionization=None and ionization=False, plus the equivalent nested BuildInput dictionary/YAML-shaped input, and assert they normalize to the same disabled state.
• Verify omitted ionization still creates the default IonizationConfig, explicit positive ion counts still follow the existing path, and nonzero counterions are not skipped when neutralization is requested.
• Verify BuildInput.metadata and to_data_row/from_data_row do not raise for disabled ionization and preserve the disabled state.
• Run the repository short test command (pytest -k "not build" mdfactory), targeted build/model tests as needed, and the configured Ruff/pre-commit checks.

Acceptance criteria

• The reported mixedbox reproduction with zero concentration and disabled neutralization completes past ionization without the empty-AtomGroup exception.
• Zero requested ions produce no NA/CL species and do not invoke the lower-level ion replacement routine.
• ionization=None and ionization=False are accepted directly in composition/build input and behave as no-ops; omitted ionization retains current defaults.
• Enabled ionization, neutralization, metadata, and serialization behavior remain backward compatible.
• Regression tests cover the new paths and the documented input forms, and the project test/lint checks pass.

Risks and open questions

• Applying the opt-out normalization to all three shared composition models is slightly broader than the reported mixedbox failure, but keeps the public shared ionization setting consistent with the existing build helper and documentation.
• The lower-level setup.solvation.ionize function will retain its existing positive-count contract; the wrapper must prevent zero-count calls from the supported build inputs. A charged system with zero salt concentration must still receive its required counterions, so the no-op condition will require both requested counts to be zero.

---

Plan created by mach6

[maxscheurer]

Progress Update

Implemented the planned ionization fixes:

• Added a zero-ion early return in ionize_solvated_system, preserving the input universe and avoiding the empty MDAnalysis merge.
• Added shared IonizationConfig | None handling with literal false normalization for mixedbox, bilayer, and LNP inputs.
• Made metadata serialization safe for disabled ionization.
• Added regression/model round-trip tests and documented null/false opt-out forms.

Verification:

• Targeted regression and model tests passed.
• The short suite reached 2,946 passed, 4 skipped, and 1 unrelated existing failure in test_lock_folder_processes caused by Python 3.14 multiprocessing pickling of a local function.
• Ruff pre-commit hooks passed for the committed files.

Commit: 969fe69

---

Progress tracked by mach6

[maxscheurer]

Unverified Review Candidates — Pending Assessment

Review round: 1
Reviewed commit: 969fe69

These are unverified candidates. Severity reflects reviewer confidence; do not treat any item as a merge blocker until the assessment comment is posted.

Critical

Finding 1 — Counterion-only neutralization still passes an empty ion group to MDAnalysis.Merge (99 confidence)

mdfactory/build.py:672-680 correctly skips the both-zero case, but charged systems at zero salt have exactly one nonzero ion count and still call ionize(). mdfactory/setup/solvation.py:291-296 always calls mda.Merge(atomstokeep, cations, anions), including the empty group. This can reproduce the same empty-AtomGroup exception for one-sided counterion placement and violates the requirement to preserve neutralization whenever either count is nonzero.

Important

Finding 2 — The configured zero-ion no-op still requires recognized water (94 confidence)

mdfactory/build.py:659-674 checks the water selection before the both-zero return. A dry system, or one whose solvent is not recognized by MDAnalysis' water selection, therefore raises ValueError("Cannot ionize system without water.") even when concentration=0.0, neutralize=False, and no ions need placement. This differs from explicit ionization: false and from the documented statement that this configuration leaves the system unchanged.

Suggestions

Finding 3 — Charged zero-salt neutralization lacks regression coverage (96 confidence)

The new test at mdfactory/tests/test_build.py:59-68 covers only the both-zero branch. It does not verify that positive or negative charged systems with zero salt still invoke ionize() with the required one-sided counterions. This compatibility path is explicitly included in the approved plan and would expose finding 1.

Finding 4 — Disabled ionization tests cover only mixedbox (88 confidence)

The public setting was extended to mixedbox, bilayer, and LNP, and the bilayer metadata branch changed independently, but mdfactory/tests/test_models.py:233-265 exercises only mixedbox. Bilayer/LNP BuildInput dispatch and serialization regressions could therefore escape the new tests.

Strengths

• Explicit None and literal False normalize cleanly to one canonical disabled state while omission retains the existing default.
• The both-zero path preserves universe identity, reports no additional species, and avoids the lower-level replacement routine.
• Metadata and data-row serialization safely preserve disabled ionization.
• Documentation clearly distinguishes disabling, omission, and configured zero-ion behavior.
• The implementation centralizes normalization and introduces no unnecessary abstraction or style churn.

Agents run: code-reviewer, error-auditor, test-reviewer, completeness-checker, simplifier

---

Reviewed by mach6

[maxscheurer]

Review Assessment

#43 (comment)

Classifications

Finding	Classification	Reasoning
Finding 1	merge blocker	Factual: charged zero-salt systems produce exactly one nonzero ion count, while ionize() unconditionally passes both cation and anion groups to MDAnalysis.Merge; one is empty. Scope: the approved plan explicitly requires counterion behavior whenever either count is nonzero and backward-compatible neutralization. Practical: a supported charged mixedbox, bilayer, or LNP build with zero salt can abort instead of being neutralized.
Finding 2	merge blocker	Factual: water selection and its error occur before the both-zero return. Scope: the plan and new documentation require configured zero concentration with disabled neutralization to be a no-op. Practical: a dry or unrecognized-solvent mixedbox build aborts even though no solvent replacement is requested; moving the no-op decision ahead of water validation preserves the water error when ions are actually needed.
Finding 3	merge blocker	Factual: the new test covers only an uncharged both-zero request and cannot exercise one-sided replacement. Scope: the approved testing approach explicitly requires verifying that nonzero counterions are not skipped. Practical: the missing charged zero-salt regression allows the build-crashing defect in finding 1 to pass the suite; positive and negative charge cases should prove correct one-sided placement and species counts.
Finding 4	useful follow-up	Factual: new disabled-ionization tests instantiate only mixedbox, despite independent bilayer metadata code and shared LNP exposure. Scope: all three models are in scope, but the plan does not unambiguously require the full model/value matrix. Practical: current shared typing works and no present failure is demonstrated; model-specific tests would mainly guard future regressions.

Action Plan

1. Make mdfactory/setup/solvation.py support sodium-only and chloride-only replacement without passing empty groups to MDAnalysis.Merge; return only species actually added.
2. Return from the configured both-zero path before requiring a recognized water selection, while retaining the water error whenever ions are requested.
3. Add charged zero-salt regression tests for one-sided counterion placement, preferably both charge signs, asserting successful ionization and correct species/counts.

---

Assessment by mach6

[maxscheurer]

Progress Update

Resolved review findings 1–3:

• Updated ion replacement to omit empty sodium or chloride groups from MDAnalysis.Merge, allowing counterion-only neutralization.
• Moved the configured zero-ion no-op ahead of water validation, so dry systems remain unchanged when no ions are requested.
• Report only ion species with positive counts.
• Added regressions for dry-system no-op behavior and both positive- and negative-charge zero-salt neutralization.

Verification:

• Focused ionization tests passed: 4 passed.
• Existing two-sided ionization test passed.
• Short suite: 2,946 passed, 4 skipped, 185 deselected, with the known unrelated Python 3.14 multiprocessing pickling failure in test_lock_folder_processes.
• Ruff/pre-commit formatting and lint checks passed for the committed changes.

Commit: 99c0aef

---

Progress tracked by mach6

[MSiggel]

LGTM