docs: four comments that misdescribe the code beside them (3.2.3, 3.2.4, 3.2.5, 3.5.3)

[jdeast]

Batch 3C of the review sweep: four places where a comment, docstring or error
message misdescribes the code beside it. In all four the code is right and only
the text changed. No expression was touched.

All four premises held on the current tree. Every anchor had drifted and every one
was re-verified against HEAD before editing.

---

3.2.3 -- OwnPrePatchRef docs overstated the guard

What the text claimed. The class docstring (parameter.py:570-586): build_pymc
"refuses a reference to any non-sampled element". The inline comment: the pre-patch
tensor "is only final on SAMPLED elements". The error text: the referenced elements
"are not SAMPLED".

What the code actually does. The guard is

_non_sampled = self._inactive_mask(n) | (union of every expression mask)

i.e. inactive | derived | reported. It is not ~is_sampled, so a sigma: 0
pin passes -- and correctly so.

How I confirmed it. Read the guard's construction, then traced what a fixed
element's pre-patch slot actually holds:

• is_fixed = ((sigmas == 0) | is_inactive) & ~is_derived;
• the transform loop at :1760 opens with if not is_sampled[i]: continue, so a
  fixed element keeps use_logit[i] == False and gaussian_mus[i] == inits[i]
  (from the np.copy(inits) initializer);
• its raw is pt.constant(0.0) (:1876), so the linear branch yields
  gaussian_mus[i] + gaussian_scales[i]*0 == inits[i] -- exactly its pin;
• the pin rule at :1523 already guarantees that number exists for an active pin.

A fixed element's pre-patch slot is therefore final, and the review's reading is right.

Corrected wording. All four sites now agree -- the fourth being
component.py:784, which made the same claim and is corrected for consistency. They
now say: the pre-patch tensor is final on sampled and fixed elements; what is
refused is a derived, reported or inactive element, whose slot holds the
transform's placeholder or a bookkeeping pin. The docstring adds that _non_sampled
should be read as "not yet final", not as "not sampled", so the variable name
cannot re-seed the same misreading.

One residual edge found while verifying, recorded rather than "fixed". A
HARD-LINKED element is sigma: 0 too, but section 5b writes its value (:2155)
after 5a's patch (:2054), so its pre-patch slot holds its resolved initval rather
than the link's value -- the guard does not cover it. Nothing combines a hard link
with a same-parameter element dep today. Widening the mask would be a behavior
change, so the docstring names it and says to widen the mask rather than trust the
paragraph if something ever does.

No test asserted the old error string (grep over tests/ for "not SAMPLED" /
"pre-patch" finds only prose in test_fitmurel.py and test_vcve.py, neither an
assertion).

---

3.2.4 -- constants.py CORE_FRACTION comment went stale in PR #215

What the text claimed. "run.py resolves cores as
max(1, min(int(n_phys * CORE_FRACTION), n_phys - 1))".

What the code actually does. That formula now has exactly one spelling,
samplers/_common.py:296-315's default_cores(), and run.py:379 just calls it:

cores = int(_cores_raw) if _cores_raw is not None else default_cores()

How I confirmed it. grep -n default_cores src/exozippy/run.py -- imported at
:21, called at :379 and :1973 -- and read default_cores()'s body and
docstring, which records the same three-copy collapse from #215.

Corrected wording. The comment now points at default_cores() as the one
spelling and names the three copies #215 collapsed into it (run.py's inline formula,
create_pool's and nested.py's hardcoded 0.75, the last having dropped the
n_phys - 1 arm entirely). That keeps the constants block's own rationale -- "cannot
find the other place that has to move with it" -- true, which it only is if it names
the right other place.

It deliberately says nothing about what a user's cores: value means downstream:
review 2.4.8 has cores=0 meaning three different things across the three
resolvers, so the comment claims only that a sampler: cores: key is passed through
by run.py and bypasses this constant, and that cores=None means AUTO -- which is
default_cores' own documented contract.

---

3.2.5 -- _get_conversion_factors' failure message stated the conversion BACKWARDS

Filed PLAUS, and the one item here that touches the reciprocal-factor trap CLAUDE.md
devotes an invariant to. The premise held.

What the text claimed vs. what the code does. The message read
"Conversion failure from '{u_str}' to '{i_str}'" -- user -> internal -- while the
operation it reports is internal -> user.

How I confirmed the direction, three independent ways (getting this backwards a
second time would be worse than leaving it):

1. The operative call. return float(self.internal_unit.to(target_u)), where
   target_u is the USER unit. astropy's Unit.to(other) returns the multiplier
   taking a value expressed in self to one expressed in other, so the factor is
   internal -> user. Measured in this venv:
   Unit('solMass').to(Unit('jupiterMass')) == 1047.5655146604772, and
   (1*solMass).to(jupiterMass) == 1047.5655146604772 jupiterMass -- the factor takes
   an internal-unit number to a user-unit number.
2. Both directional wrappers have to be right. from_internal (INTERNAL -> USER)
   multiplies by this factor; to_internal divides. Only an internal -> user
   factor makes both correct.
3. Three existing statements already agreed and the message was the lone
   dissenter: the function's own docstring ("the numerical conversion factor from
   internal -> user units"), the DIRECTION block above element_factor (:3155-3163),
   and the CLAUDE.md invariant. That the message disagreed with its own docstring two
   lines up is itself the evidence that the message, not the code, was wrong.

The advice was wrong too. "Ensure units are valid astropy strings" misdiagnoses
the case that reaches this branch: both units can be valid and simply not
convertible. Measured: Unit('dex') and Unit('solMass') are both valid and
.to() between them raises UnitConversionError: 'dex' and 'solMass' (mass) are not convertible. The dex short-circuit above only fires when BOTH sides are dex, so a dex
internal unit against a linear user one lands here.

Corrected wording.

f"[{self.label}] Conversion failure from '{i_str}' to '{u_str}'. "
f"Either a unit is not a valid astropy string, or the two are "
f"dimensionally incompatible (e.g. a dex internal unit against "
f"a linear user one). Original error: {e}"

plus a comment at the raise recording which direction the message states and why, so
the next reader does not have to re-derive it. Nothing in src, tests or docs
matched "Conversion failure", so no test pinned the old string.

---

3.5.3 -- transit.py cited a nonexistent _reset_build_caches

What the text claimed. _build_dilution's docstring: "build_likelihood
clears it (see _reset_build_caches)".

What the code actually does. _reset_build_caches exists nowhere --
grep -rn '_reset_build_caches' src tests docs returns that one comment and nothing
else. The real mechanisms are two:

• the inline self._dilution_node = None at the top of build_likelihood
  (components/transit/transit.py:549), which is what actually clears this cache;
• the general seam Component.per_build_caches / reset_build_caches
  (component.py:319-324), run by System.build_model before stage 5
  (system.py:782).

How I confirmed it. Read both, and confirmed Transit does not declare
per_build_caches -- only SED does (sed.py:733) -- so the inline reset really is
this cache's mechanism.
tests/test_rebuild_caches.py::test_build_likelihood_drops_a_stale_dilution_node
pins the behavior.

Corrected wording. The docstring now names the inline reset and points at the
general contract, including why this cache does not use it: nothing reads the node
before stage 7, which is exactly the distinction components.md already draws
against SED._m_pred_matrix (built at stage 6, so a stage-7 reset would come too
late).

---

Acceptance: start logp is bit-identical

Ran the full acceptance pair before and after the edits on the same machine:

• scripts/make_mulens_fixtures.py --check -- every per-term logp and every fixture
  verdict reproduced identically.
• EXOZIPPY_ACCEPTANCE_STRICT=1 pytest tests/test_mulens_acceptance.py -- 24
  passed on the strict 1e-14 tier. Sign tally over 925 terms: 923 bit-identical,
  1 above and 1 below, max |rel| 5.4e-16 -- the same two 1-ulp terms with the same
  values before and after.

Diffing the numeric and verdict lines of the two runs: 38 lines, byte-identical.
The only textual difference between the two logs is pytensor compile-cache
housekeeping warnings and the pytest timing line.

AST-level confirmation of "no behavior change". Parsed master's and this
branch's version of all four touched files and compared the ASTs with string
constants normalized and docstrings stripped. component.py, transit.py and
constants.py are structurally identical. parameter.py differs in exactly two
functions -- _get_conversion_factors (and _process_single, its nested closure) and
build_pymc -- and in each, exactly one f-string differs with the f-string count
unchanged (1 -> 1 and 28 -> 28). Those two f-strings are the two error messages this
PR intends to change. Nothing else executable moved.

parameter.md was checked for a contradiction and needs no amendment: it does not
document the OwnPrePatchRef guard condition at all, so nothing in it demanded the
stricter refusal, exactly as item 3.2.3 anticipated.

---

Two pre-existing issues found, NOT from this PR (worth filing)

1. make_mulens_fixtures.py --check already exits 1 on master at 78a1141,
   identically before and after this change: ob09020's
   POT:galacticmodel.imf_prior differs by 1 ulp (2.2e-16 absolute, rel 1.3e-16),
   OGLE_0383LD's RV:mulensinstrument.model by 1 ulp (rel 5.4e-16), and
   ob09020_accept, ob09020_diag, ob09020_polish, ob09020_tune report MISSING
   fixture. Both deltas are far inside the strict 1e-14 tier, which is why
   test_mulens_acceptance.py is green. As it stands --check cannot be used as a
   clean pass/fail gate.

2. Two concurrent suites corrupt each other's pytensor compiledir. A full-suite
   run of this branch produced 13 failures and 4 errors, every one
   pytensor CompileError with
   /bin/ld: cannot open output file .../pytensor-pytest/gwN/compiledir_.../tmpXXXX/....so: No such file or directory -- a temp compile directory removed mid-link. Cause:
   EXOZIPPY_TEST_COMPILEDIR is shared, conftest appends only the worker id (gwN),
   and the controller prunes LRU and sweeps sibling platform trees on the stated
   rationale that "this base_compiledir belongs to the test suite alone" -- an
   assumption the design otherwise breaks deliberately, since the cache is shared
   across worktrees so parallel agents get a warm one. Re-running those 10 files
   against a private compiledir: 213 passed, 0 failed. So the suite is green for
   this branch, but the shared-compiledir race makes a concurrent run look like 13
   real breakages.

🤖 Generated with Claude Code