Retire claims that #55 and #93 outlived

NEWS.md

@@ -1,5 +1,28 @@
 # phylloptim 0.5.0
 
+## Documentation: claims that #55 and #93 outlived
+
+No behaviour change. Four places still described the model as it was before two
+merged fixes, and one of them was user-facing advice:
+
+- `vignettes/fitting.Rmd` told readers that a bare `l$vcmax_25 <- x` followed by
+  re-driving takes a temperature-cache **hit** and silently reports numbers from the
+  first `vcmax` it ever saw. #55 widened the cache key to every input of the
+  temperature block, so that is no longer true. The advice — use `set_traits()` — is
+  unchanged; the reasons are now the splines, the solved operating point and the #25
+  checks, which no re-driving repairs.
+- `inst/RcppR6_classes.yml` carried the same claim, contradicting the correct
+  statement 150 lines above it in the same file.
+- `tests/cpp/test_leaf.cpp` claimed a sub-assertion isolated `set_traits`' cache
+  invalidation. It cannot any more: with `vcmax_25` in the key the cache misses
+  regardless, so the trap is closed twice over and the test cannot say which
+  mechanism carried it. The two halves are covered separately, and it now says so.
+- `inst/include/phylloptim/closed_form.hpp` divides by `atm_vpd_` throughout, which
+  #93 replaced with the leaf-to-air `vpd_leaf_` in the live model. Recorded rather
+  than repaired: it is dormant, PM-untested code, and the substitution may not be a
+  rename — `vpd_leaf_` depends on Tleaf, which depends on the E these expressions
+  solve for.
+
 ## `lambda_` is a caller input, so nothing resets it (#96)
 
 `setup_clean_leaf()` cleared `lambda_`, so whether a prescribed Sperry marginal water

inst/RcppR6_classes.yml

@@ -380,9 +380,18 @@ Leaf:
     # A METHOD, NOT FIFTEEN SETTABLE FIELDS. The traits are public doubles, so
     # `access: field` entries would have worked and would have been wrong: a bare
     # field write bypasses the #25 invariant checks, leaves the two pre-integrated
-    # vulnerability splines describing the previous curve, and -- worst -- leaves
-    # vcmax_/jmax_/R_d_ stale, because set_physiology derives them behind a cache
-    # keyed on (leaf_temp, atm_o2_kpa) alone and so takes the hit on the next call.
+    # vulnerability splines describing the previous curve, and leaves the solved
+    # operating point describing the old traits (hazard 8).
+    #
+    # This used to carry a third reason -- that vcmax_/jmax_/R_d_ would go stale
+    # behind a temperature cache keyed on (leaf_temp, atm_o2_kpa) alone, so that
+    # re-supplying the drivers took a cache HIT and never recomputed. **#55 closed
+    # that**: the key is every scalar `update_temperature_dependent_params()`
+    # reads, `vcmax_25` and `jmax_25` among them, exactly as the temperature-response
+    # block above already says. The two reasons that remain are enough, and this
+    # comment contradicting that one for two releases is why the claim is stated in
+    # one place now.
+    #
     # The leaf_model.hpp declaration has the full reasoning; it is the same
     # judgement as supply_kind above, reached from a different direction.
     #

inst/include/phylloptim/closed_form.hpp

@@ -32,6 +32,19 @@
 // within_guard below and PLAN.md item 9) before it can replace the exact solve on
 // a production path.
 //
+// ⚠️ AND IT HAS DRIFTED FROM THE SOLVE IT APPROXIMATES. Every `D` below is
+// `atm_vpd_`, the deficit at AIR temperature, because that is what Fick's law
+// divided by when this was written. #93 moved the live model to `vpd_leaf_`, the
+// leaf-to-air deficit, which on the energy-balance path is 3-4x the air's over
+// Tair 25-45 -- so on that path these formulae now approximate a model that no
+// longer exists. Off it the two are equal by construction (`set_leaf_vpd` returns
+// `atm_vpd_` exactly), so the prescribed-temperature path is unaffected.
+//
+// Not repaired here: it is a numerics change to dormant, PM-untested code, and the
+// substitution is not obviously just a rename -- `vpd_leaf_` depends on Tleaf,
+// which depends on E, which is what these expressions solve for, so the closed
+// form may not stay closed. Filed rather than guessed.
+//
 // FOUR THINGS NOT TO GET WRONG, all learned the expensive way over there:
 //
 //  1. `newton_steps = 1` is deliberate. Two steps are *worse* in the tail.

tests/cpp/test_leaf.cpp

@@ -2770,19 +2770,34 @@ void test_set_traits_matches_a_fresh_leaf() {
        "transpiration is bit-identical" + what);
   }
 
-  // The specific trap, isolated, because the bit-exact comparisons above would
-  // also pass if set_traits rebuilt everything unconditionally and the reason it
-  // works were lost. vcmax_ is derived inside set_physiology's temperature cache,
-  // which is keyed on (leaf_temp, atm_o2_kpa) and NOT on the traits -- so this is
-  // the assertion that "change the trait, then set the drivers again" is a
-  // sufficient recipe, which it is only because set_traits invalidates the cache.
+  // "Change the trait, then set the drivers again" really does recompute the
+  // temperature block, checked end to end through the route a caller takes.
+  //
+  // ⚠️ THIS NO LONGER ISOLATES set_traits' CACHE INVALIDATION, and the comment
+  // here claimed it did for two releases. It said vcmax_ is derived behind a cache
+  // "keyed on (leaf_temp, atm_o2_kpa) and NOT on the traits", so the recipe worked
+  // "only because set_traits invalidates the cache". #55 widened the key to every
+  // scalar update_temperature_dependent_params() reads -- `vcmax_25` included --
+  // so the trap is now closed TWICE OVER: the key would miss on the changed trait
+  // even if set_traits cleared nothing. The assertion below cannot tell which
+  // mechanism carried it, and pretending otherwise is how a test comes to describe
+  // a guarantee it is not testing.
+  //
+  // The two halves are covered separately and deliberately:
+  //   * the KEY -- test_temperature_params_invalidate_cache, which sets a response
+  //     parameter and re-drives without going through set_traits at all;
+  //   * set_traits' CLEARING -- the bit-exact comparisons above (the splines and
+  //     the solved operating point are not in any key), plus the NA assertions in
+  //     test_prescribed_lambda_survives_redriving.
   {
     phylloptim::Leaf l = make_leaf(d, {2.0}, {1.0});
     const double vcmax_before = l.vcmax_;
     l.set_traits(96.0 * 2.0, 2.680147, 3.898245, 5.870283, 2.680147, 3.898245,
                  5.870283, 1.5, 157.44, 0.30, 0.7, 0.99, 7.5, kRd25);
     std::vector<double> mrp{1.0 / d.area_leaf}, psi_soil{2.0}, depth{1.0};
-    // The SAME leaf_temp and atm_o2_kpa, which is what arms the cache.
+    // The same leaf_temp and atm_o2_kpa -- which used to be the whole key, and so
+    // used to be what ARMED the trap. It no longer is: `vcmax_25` has moved, so the
+    // key differs whatever set_traits did.
     l.set_physiology(fixture::root_network(mrp, depth), d.PPFD, psi_soil, depth, d.K_s * d.theta / d.h,
                      d.atm_vpd, d.ca, d.leaf_temp, d.atm_o2_kpa, d.atm_kpa);
     near(l.vcmax_ / vcmax_before, 2.0, 1e-12,

vignettes/fitting.Rmd

@@ -80,12 +80,23 @@ on an existing object; constructing a fresh one costs ~204 µs against ~4 µs to
 re-trait, which is 73 solves thrown away per evaluation. In a fit that dominates
 everything else.
 
-⚠️ **And do not write `l$vcmax_25 <- x` instead.** It compiles and is wrong: the
-derived `vcmax_`, `jmax_` and `R_d_` live in a temperature cache keyed on
-`(leaf_temp, atm_o2_kpa)` and *nothing else*, so changing a trait and re-setting
-the drivers takes a cache **hit** and never recomputes them. A sweep written that
-way runs at the first `vcmax` it ever saw and reports plausible numbers
-throughout. `set_traits()` refreshes all of it.
+⚠️ **And do not write `l$vcmax_25 <- x` instead.** It compiles and is wrong, though
+not for the reason this vignette used to give. Two pieces of derived state describe
+the old trait afterwards and no re-driving repairs either: the **pre-integrated
+vulnerability splines** (`stem_b`/`stem_c` and `root_b`/`root_c` each own one), and
+the **solved operating point**, which no code path rewrites — so it goes on reading
+as though it belonged to the new traits. A bare write also bypasses the
+positive-magnitude checks on `psi_crit` and the two `b` parameters.
+
+`set_traits()` clears all of it, and is the only route bound to R.
+
+*Corrected in 0.5.0:* this warning used to say the derived `vcmax_`, `jmax_` and
+`R_d_` sat behind a temperature cache keyed on `(leaf_temp, atm_o2_kpa)` and
+*nothing else*, so that re-supplying the drivers took a cache **hit** and silently
+kept the old response. That was true and is not: since the cache key became every
+input of the temperature block — `vcmax_25` and `jmax_25` among them — re-driving
+after a bare trait write *does* recompute those three. The advice is unchanged; the
+reason for it is narrower.
 
 ```{r}
 l <- leaf_model(truth)