More Matt review

contrib/src/main/java/gov/nasa/jpl/aerie/contrib/streamline/core/CellRefV2.java

@@ -1,8 +1,6 @@
 package gov.nasa.jpl.aerie.contrib.streamline.core;
 
 import gov.nasa.jpl.aerie.contrib.streamline.core.monads.ErrorCatchingMonad;
-// MD: Remove unused import
-import gov.nasa.jpl.aerie.contrib.streamline.debugging.Naming;
 import gov.nasa.jpl.aerie.merlin.framework.CellRef;
 import gov.nasa.jpl.aerie.merlin.protocol.model.CellType;
 import gov.nasa.jpl.aerie.merlin.protocol.model.EffectTrait;

contrib/src/main/java/gov/nasa/jpl/aerie/contrib/streamline/core/Reactions.java

@@ -3,8 +3,6 @@
 import gov.nasa.jpl.aerie.contrib.streamline.modeling.discrete.Discrete;
 import gov.nasa.jpl.aerie.merlin.framework.Condition;
 import gov.nasa.jpl.aerie.merlin.protocol.types.Duration;
-// MD: Unused import
-import org.apache.commons.lang3.mutable.MutableObject;
 
 import java.util.function.Consumer;
 import java.util.function.Supplier;

contrib/src/main/java/gov/nasa/jpl/aerie/contrib/streamline/core/Resources.java

@@ -16,10 +16,7 @@
 import static gov.nasa.jpl.aerie.contrib.streamline.core.MutableResource.resource;
 import static gov.nasa.jpl.aerie.contrib.streamline.core.Expiring.neverExpiring;
 import static gov.nasa.jpl.aerie.contrib.streamline.core.Expiry.NEVER;
-// MD: Unused imports
-import static gov.nasa.jpl.aerie.contrib.streamline.core.Reactions.whenever;
 import static gov.nasa.jpl.aerie.contrib.streamline.core.Reactions.wheneverDynamicsChange;
-import static gov.nasa.jpl.aerie.contrib.streamline.core.monads.ResourceMonad.map;
 import static gov.nasa.jpl.aerie.contrib.streamline.debugging.Dependencies.addDependency;
 import static gov.nasa.jpl.aerie.contrib.streamline.debugging.Naming.*;
 import static gov.nasa.jpl.aerie.contrib.streamline.modeling.clocks.Clock.clock;
@@ -241,6 +238,9 @@ public static <D extends Dynamics<?, D>> Resource<D> cache(Resource<D> resource)
    *   can cause discrete changes in the result.
    *   For example, imagine a continuous numeric resource R,
    *   and a derived resource S := "R > 0".
+   *
+  // MD: Huh, that's an interesting limitation. Can this be achieved with expiry?
+
    *   If R changes continuously from positive to negative,
    *   then S changes discretely from true to false, *without* an effect.
    *   If used directly, Aerie would not re-sample S at this time.
@@ -259,6 +259,7 @@ public static <D extends Dynamics<?, D>> Resource<D> cache(Resource<D> resource)
   // REVIEW: Suggestion from Jonathan Castello to remove this method
   // in favor of allowing resources to report expiry information directly.
   // This would be cleaner and potentially more performant.
+  // MD: What would that look like?
   public static <D extends Dynamics<?, D>> Resource<D> signalling(Resource<D> resource) {
     var cell = resource(discrete(Unit.UNIT));
     name(cell, "Signal for (%s)", resource);
@@ -274,6 +275,7 @@ public static <D extends Dynamics<?, D>> Resource<D> signalling(Resource<D> reso
   }
 
   public static <D extends Dynamics<?, D>> Resource<D> shift(Resource<D> resource, Duration interval, D initialDynamics) {
+    // MD: What happens if `interval` is negative?
     var cell = resource(initialDynamics);
     delayedSet(cell, resource.getDynamics(), interval);
     wheneverDynamicsChange(resource, newDynamics ->

contrib/src/main/java/gov/nasa/jpl/aerie/contrib/streamline/debugging/Naming.java

@@ -33,7 +33,7 @@ private Naming() {}
    * Register a name for thing, as a function of args' names.
    * If any of the args are anonymous, so is this thing.
    */
-  public static void name(Object thing, String nameFormat, Object... args) {
+  public static <T> T name(T thing, String nameFormat, Object... args) {
     // Only capture weak references to arguments, so we don't leak memory
     var args$ = Arrays.stream(args).map(WeakReference::new).toArray(WeakReference[]::new);
     NAMES.put(thing, () -> {
@@ -49,6 +49,7 @@ public static void name(Object thing, String nameFormat, Object... args) {
       }
       return Optional.of(nameFormat.formatted(argNames));
     });
+    return thing;
   }
 
   /**

contrib/src/main/java/gov/nasa/jpl/aerie/contrib/streamline/modeling/Registrar.java

@@ -108,9 +108,8 @@ public void setProfile() {
     profile = true;
   }
 
-  // MD: This one should probably be profile = false;
   public void clearProfile() {
-    profile = true;
+    profile = false;
   }
 
   public <Value> void discrete(final String name, final Resource<Discrete<Value>> resource, final ValueMapper<Value> mapper) {

contrib/src/main/java/gov/nasa/jpl/aerie/contrib/streamline/modeling/polynomial/LinearBoundaryConsistencySolver.java

@@ -42,6 +42,8 @@
 import static java.util.function.Function.identity;
 import static java.util.stream.Collectors.toMap;
 
+// MD: Mkay, this file will probably need a few hours of poring over.
+
 /**
  * Special methods for setting up a substepping resource solver
  * using linear constraints and arc consistency.

contrib/src/main/java/gov/nasa/jpl/aerie/contrib/streamline/modeling/polynomial/Polynomial.java

@@ -28,6 +28,7 @@
 public record Polynomial(double[] coefficients) implements Dynamics<Double, Polynomial> {
 
   // TODO: Add Duration parameter for unit of formal parameter?
+  // MD: Is the convention to provide coefficients from least significant to most significant? Can we document that?
   public static Polynomial polynomial(double... coefficients) {
     int n = coefficients.length;
     if (n == 0) {
@@ -38,6 +39,7 @@ public static Polynomial polynomial(double... coefficients) {
       // Any NaN coefficient invalidates the whole polynomial
       if (Double.isNaN(coefficients[m])) return new Polynomial(new double[] { Double.NaN });
       // Infinite coefficients invalidate later terms
+      // MD: Infinite coefficients blow my mind a bit - could you help me understand why they don't invalidate the whole polynomial?
       if (Double.isInfinite(coefficients[m])) {
         n = m + 1;
         break;
@@ -151,6 +153,8 @@ public double evaluate(Duration t) {
    * Helper method for other comparison methods.
    * Finds the first time the predicate is true, near the next root of this polynomial.
    */
+  // MD: Why would the expiry be correlated with the roots of this polynomial?
+  // MD: If the predicate returns true before the next root..? Will that expiry be missed?
   private Expiry findExpiryNearRoot(Predicate<Duration> expires) {
     Duration root, start, end;
     try {

contrib/src/main/java/gov/nasa/jpl/aerie/contrib/streamline/unit_aware/Unit.java

@@ -26,6 +26,7 @@ private Unit(final Dimension dimension, final double multiplier, final String lo
 
   public static Unit createBase(final String shortName, final String longName, final Dimension dimension) {
     // TODO: Track base units, to detect and prevent collisions
+    // MD: Should this be in a separate PR?
     assert(dimension.isBase());
     return new Unit(dimension, 1, longName, shortName);
   }

examples/streamline-demo/src/main/java/gov/nasa/jpl/aerie/streamline_demo/DataModel.java

@@ -51,6 +51,8 @@ public DataModel(final Registrar registrar, final Configuration config) {
     var correctedVolumeA = map(clampedVolumeA, actualRateA, (v, r) -> r.integral(v.extract()));
     var correctedVolumeB = map(clampedVolumeB, actualRateB, (v, r) -> r.integral(v.extract()));
     var correctedVolumeC = map(clampedVolumeC, actualRateC, (v, r) -> r.integral(v.extract()));
+
+    // MD: Why is eraseExpiry necessary here?
     // Use the corrected integral values to set volumes, but erase expiry information in the process to avoid loops:
     forward(eraseExpiry(correctedVolumeA), this.volumeA);
     forward(eraseExpiry(correctedVolumeB), this.volumeB);

examples/streamline-demo/src/main/java/gov/nasa/jpl/aerie/streamline_demo/ErrorTestingModel.java

@@ -10,6 +10,7 @@
 import gov.nasa.jpl.aerie.contrib.streamline.modeling.polynomial.Polynomial;
 import gov.nasa.jpl.aerie.contrib.streamline.modeling.polynomial.PolynomialResources;
 
+import static gov.nasa.jpl.aerie.contrib.streamline.debugging.Naming.name;
 import static gov.nasa.jpl.aerie.contrib.streamline.modeling.discrete.DiscreteResources.discreteResource;
 import static gov.nasa.jpl.aerie.contrib.streamline.modeling.discrete.monads.DiscreteResourceMonad.map;
 import static gov.nasa.jpl.aerie.contrib.streamline.modeling.polynomial.PolynomialResources.*;
@@ -23,7 +24,7 @@ public class ErrorTestingModel {
       asPolynomial(map(counter, c -> (double) c)),
       asPolynomial(map(bool, $ -> $ ? 1.0 : -1.0)));
 
-  public MutableResource<Polynomial> upperBound = PolynomialResources.polynomialResource(5);
+  public MutableResource<Polynomial> upperBound = name(PolynomialResources.polynomialResource(5), "upperBound");
   public MutableResource<Polynomial> lowerBound = PolynomialResources.polynomialResource(-5);
   public Resource<Polynomial> clamped = clamp(constant(10), lowerBound, upperBound);