WIP - Adding resource builders

This commit and the one before it attempt to coalesce building a resource and registering it, since you need similar information for both operations.

I really like one thing about this approach: Discrete primitive resources get value mappers by default, meaning they "just work" with little to no additional configuration.

There are a few things I don't like about this approach:
1. Writing the builder is tedious and repetitive, since I'm writing one for each kind of resource (discrete, Polynomial, clock, etc.).
   It strongly feels like most of this behavior could be moved up to a higher level of abstraction, somehow.
2. This makes registration of MutableResource quite different from registration of "regular" Resources.
   Relatedly, specialized MutableResource constructors now need to either use the builder or return the builder, both of which feel wrong.
   You're either leaking an abstraction that you likely shouldn't be, or you're restricting the modeler in an awkward, artificial way by making all the builder choices for them.
   Or, you end up bloating the special-purpose constructors too much to expose all the builders' functionality.
3. Relatedly, for things like Polynomial resources, where we often register an approximation, it feels like we should handle that akin to discrete.
   But we can't, strictly, since what we register and what we return are different. Building the choice of approximation into this class feels wrong.
   It suggests that if you build the way to register into the resource construction, it ought to be general enough to apply to any resource, not just polynomial / discrete.
3. Passing the registrar to the builder now feels awkward. I think maybe the registrar should become a singleton we access at-will, like the Logger.

Alternate designs include:
1. Making a general resource builder, and letting particular kinds either sub-class that builder or write helper methods that fit the arguments of that builder.
   For example, the general builder accepts a ValueMapper<Dynamics>, and discrete resources could offer a function from ValueMapper<T> to ValueMapper<Discrete<T>>.
   The awkward bit here (maybe?) is registration, as most resources can't be registered. Only discrete and linear can be.
   However, point 3 above suggests if we expand our notion of registration to include a mapping step, maybe anything could be registered, they just aren't by default...
2. Get rid of the builder altogether, and instead lean on that "helpers that build the arguments of your general constructor" pattern *hard*.
   This is what we had in streamline up to this commit, and it works well without tons of boilerplate code to maintain.
   Maybe we could just make the registrar smarter, such that it can at least pull the name out of Naming.
   For more consistency, we could even remove the regsitrar constructor that takes a name, and ask users to directly invoke it like `register(name(r, "newName"), ...)` if they want to change the name.
   If we really wanted to lean even further in that direction, we could register the value mapper for discrete stuff like we register the name, in a third-party class like Naming, and have the registrar pull it from there.
   Then you'd say something like `register(serializable(name(r, "newName"), mapper))` to get the full behavior.
3. Maybe we could return InconBehavior<MutableResource<...>> instead of just the resource itself?
   If you do that, I think you could at least push the choice of how to save the resource out of the constructors, and make it a post-step where you choose notSaved or serialized.
   Then if you want to share information between that choice and registration, maybe DiscreteResources could offer helper methods for the most reasonable combinations?
   I.e., something that chooses serialized + registered with the same information? It still feels different from other kinds of registration in an awkward way...

After working through these options, I'm leaning towards option 1, keep a resource builder as a place to centralize saving and registration together, but lift it to MutableResource in general.
Then, methods like DiscreteResources.discreteResource could instantiate a builder with more default options chosen, but still expose the full generality.
With a little care, they might even be able to return a sub-class of the general builder, with additional convenience methods exposed...

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

@@ -88,7 +88,7 @@ static <D> InconBehavior<ErrorCatching<Expiring<D>>> serializing(String key, D d
     return serializing(key, pure(defaultValue), standardDynamicsMapper(mapper));
   }
 
-  private static <D> ValueMapper<ErrorCatching<Expiring<D>>> standardDynamicsMapper(ValueMapper<D> baseMapper) {
+  static <D> ValueMapper<ErrorCatching<Expiring<D>>> standardDynamicsMapper(ValueMapper<D> baseMapper) {
     return new ValueMapper<>() {
       @Override
       public ValueSchema getValueSchema() {

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

@@ -7,25 +7,16 @@
 import gov.nasa.jpl.aerie.merlin.protocol.types.Duration;
 
 import static gov.nasa.jpl.aerie.contrib.streamline.core.Expiring.*;
-import static gov.nasa.jpl.aerie.contrib.streamline.core.MutableResource.*;
 import static gov.nasa.jpl.aerie.contrib.streamline.core.Resources.signalling;
 import static gov.nasa.jpl.aerie.contrib.streamline.core.monads.ResourceMonad.bind;
 import static gov.nasa.jpl.aerie.contrib.streamline.modeling.discrete.Discrete.discrete;
 import static gov.nasa.jpl.aerie.contrib.streamline.modeling.discrete.DiscreteResources.not;
 import static gov.nasa.jpl.aerie.contrib.streamline.modeling.discrete.monads.DiscreteResourceMonad.map;
 import static gov.nasa.jpl.aerie.merlin.protocol.types.Duration.EPSILON;
-import static gov.nasa.jpl.aerie.merlin.protocol.types.Duration.ZERO;
 
 public final class ClockResources {
   private ClockResources() {}
 
-  /**
-   * Create a clock starting at zero time.
-   */
-  public static Resource<Clock> clock(String name) {
-    return resource(serializing(name, Clock.clock(ZERO), null /* TODO - get auto value mapper for Clock type */));
-  }
-
   public static Resource<Discrete<Boolean>> lessThan(Resource<Clock> clock, Resource<Discrete<Duration>> threshold) {
     return signalling(bind(clock, threshold, (Clock c, Discrete<Duration> t) -> {
       final Duration crossoverTime = t.extract().minus(c.extract());

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

@@ -13,13 +13,6 @@
 import static gov.nasa.jpl.aerie.contrib.streamline.modeling.discrete.DiscreteResources.constant;
 
 public class InstantClockResources {
-    /**
-     * Create an absolute clock that starts now at the given start time.
-     */
-    public static MutableResource<InstantClock> absoluteClock(Instant startTime) {
-        return resource(new InstantClock(startTime));
-    }
-
     public static Resource<InstantClock> addToInstant(Instant zeroTime, Resource<Clock> relativeClock) {
         return addToInstant(constant(zeroTime), relativeClock);
     }

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

@@ -2,6 +2,7 @@
 
 import gov.nasa.jpl.aerie.contrib.streamline.core.*;
 import gov.nasa.jpl.aerie.contrib.streamline.core.CellRefV2.CommutativityTestInput;
+import gov.nasa.jpl.aerie.contrib.streamline.debugging.Naming;
 import gov.nasa.jpl.aerie.contrib.streamline.modeling.Registrar;
 import gov.nasa.jpl.aerie.contrib.streamline.modeling.clocks.Clock;
 import gov.nasa.jpl.aerie.contrib.streamline.modeling.discrete.monads.DiscreteDynamicsMonad;
@@ -12,30 +13,32 @@
 import gov.nasa.jpl.aerie.contrib.streamline.unit_aware.Unit;
 import gov.nasa.jpl.aerie.contrib.streamline.unit_aware.UnitAware;
 import gov.nasa.jpl.aerie.contrib.streamline.unit_aware.UnitAwareResources;
+import gov.nasa.jpl.aerie.merlin.framework.Result;
 import gov.nasa.jpl.aerie.merlin.framework.ValueMapper;
 import gov.nasa.jpl.aerie.merlin.protocol.model.EffectTrait;
 import gov.nasa.jpl.aerie.merlin.protocol.types.Duration;
+import gov.nasa.jpl.aerie.merlin.protocol.types.SerializedValue;
+import gov.nasa.jpl.aerie.merlin.protocol.types.ValueSchema;
 
 import java.time.Instant;
 import java.time.temporal.ChronoUnit;
 import java.util.*;
 import java.util.function.BiPredicate;
 import java.util.function.Supplier;
 
+import static gov.nasa.jpl.aerie.contrib.serialization.rulesets.BasicValueMappers.*;
 import static gov.nasa.jpl.aerie.contrib.streamline.core.CellRefV2.autoEffects;
 import static gov.nasa.jpl.aerie.contrib.streamline.core.CellRefV2.testing;
 import static gov.nasa.jpl.aerie.contrib.streamline.core.Expiring.expiring;
 import static gov.nasa.jpl.aerie.contrib.streamline.core.Expiry.expiry;
-import static gov.nasa.jpl.aerie.contrib.streamline.core.MutableResource.resource;
-import static gov.nasa.jpl.aerie.contrib.streamline.core.MutableResource.serializing;
+import static gov.nasa.jpl.aerie.contrib.streamline.core.MutableResource.*;
 import static gov.nasa.jpl.aerie.contrib.streamline.core.Reactions.every;
 import static gov.nasa.jpl.aerie.contrib.streamline.core.Reactions.whenever;
 import static gov.nasa.jpl.aerie.contrib.streamline.core.Resources.*;
 import static gov.nasa.jpl.aerie.contrib.streamline.core.monads.ResourceMonad.bind;
 import static gov.nasa.jpl.aerie.contrib.streamline.core.monads.ResourceMonad.pure;
 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.ClockResources.clock;
 import static gov.nasa.jpl.aerie.contrib.streamline.modeling.discrete.Discrete.discrete;
 import static gov.nasa.jpl.aerie.contrib.streamline.modeling.discrete.DiscreteEffects.set;
 import static gov.nasa.jpl.aerie.contrib.streamline.modeling.discrete.monads.DiscreteResourceMonad.*;
@@ -50,11 +53,40 @@ public static <T> Resource<Discrete<T>> constant(T value) {
     return result;
   }
 
+  // General discrete resource constructor
   public static <T> DiscreteResourceBuilder<T> discreteResource(T initialValue) {
+    return new DiscreteResourceBuilder<T>().defaultValue(initialValue);
+  }
 
+  // Special cases for primitives, which (a) need a little special handling for double's effect trait,
+  // and (b) we can bake in the value mapper for them by default.
+  public static DiscreteResourceBuilder<Boolean> discreteResource(boolean initialValue) {
+    return new DiscreteResourceBuilder<Boolean>().defaultValue(initialValue).valueMapper($boolean());
+  }
+  public static DiscreteResourceBuilder<Integer> discreteResource(int initialValue) {
+    return new DiscreteResourceBuilder<Integer>().defaultValue(initialValue).valueMapper($int());
+  }
+  // Doubles are special, as they get a toleranced equality check by default for their effect trait.
+  public static DiscreteResourceBuilder<Double> discreteResource(double initialValue) {
+    return new DiscreteResourceBuilder<Double>()
+            .defaultValue(initialValue)
+            .valueMapper($double())
+            .effectTrait(autoEffects(testing(
+                    (CommutativityTestInput<Discrete<Double>> input) -> DoubleUtils.areEqualResults(
+                            input.original().extract(),
+                            input.leftResult().extract(),
+                            input.rightResult().extract()))));
+  }
+  public static DiscreteResourceBuilder<String> discreteResource(String initialValue) {
+    return new DiscreteResourceBuilder<String>().defaultValue(initialValue).valueMapper(string());
+  }
+  // SAFETY - Enums are final, so initialValue.getClass() equals E, hence this assignment is guaranteed correct.
+  @SuppressWarnings("unchecked")
+  public static <E extends Enum<E>> DiscreteResourceBuilder<E> discreteResource(E initialValue) {
+    return new DiscreteResourceBuilder<E>().defaultValue(initialValue).valueMapper($enum(initialValue.getClass()));
   }
 
-  public class DiscreteResourceBuilder<T> {
+  public static class DiscreteResourceBuilder<T> {
     private ErrorCatching<Expiring<Discrete<T>>> defaultValue;
     private String name;
     private ValueMapper<T> valueMapper;
@@ -66,74 +98,89 @@ public DiscreteResourceBuilder<T> defaultValue(T defaultValue) {
     }
 
     public DiscreteResourceBuilder<T> defaultValue(ErrorCatching<Expiring<Discrete<T>>> defaultValue) {
-      assertNotSet("default value", this.defaultValue);
       this.defaultValue = defaultValue;
       return this;
     }
 
     public DiscreteResourceBuilder<T> name(String name) {
-      assertNotSet("name", this.name);
       this.name = name;
       return this;
     }
 
     public DiscreteResourceBuilder<T> valueMapper(ValueMapper<T> valueMapper) {
-      assertNotSet("value mapper", this.valueMapper);
       this.valueMapper = valueMapper;
       return this;
     }
 
+    public DiscreteResourceBuilder<T> notSaved() {
+      assertSet("default value", defaultValue);
+      return inconBehavior(notSaving(defaultValue));
+    }
+
+    public DiscreteResourceBuilder<T> serialized() {
+      assertSet("name", name);
+      assertSet("default value", defaultValue);
+      assertSet("value mapper", valueMapper);
+      return inconBehavior(serializing(name, defaultValue, standardDynamicsMapper(standardDiscreteMapper(valueMapper))));
+    }
+
+    public static <T> ValueMapper<Discrete<T>> standardDiscreteMapper(ValueMapper<T> mapper) {
+      return new ValueMapper<>() {
+        @Override
+        public ValueSchema getValueSchema() {
+          return mapper.getValueSchema();
+        }
+
+        @Override
+        public Result<Discrete<T>, String> deserializeValue(SerializedValue serializedValue) {
+          return mapper.deserializeValue(serializedValue).mapSuccess(Discrete::discrete);
+        }
+
+        @Override
+        public SerializedValue serializeValue(Discrete<T> value) {
+          return mapper.serializeValue(value.extract());
+        }
+      };
+    }
+
     public DiscreteResourceBuilder<T> inconBehavior(InconBehavior<ErrorCatching<Expiring<Discrete<T>>>> inconBehavior) {
-      assertNotSet("incon behavior", this.inconBehavior);
       this.inconBehavior = inconBehavior;
       return this;
     }
 
-    private void assertNotSet(String name, Object thing) {
-      if (thing != null)
-        throw new IllegalStateException(String.format("%s has already been set on this builder!", name));
+    public DiscreteResourceBuilder<T> effectTrait(EffectTrait<DynamicsEffect<Discrete<T>>> effectTrait) {
+      this.effectTrait = effectTrait;
+      return this;
+    }
+
+    private void assertSet(String name, Object thing) {
+      if (thing == null)
+        throw new IllegalStateException(String.format("%s has not been set on this builder!", name));
     }
 
     // Terminal methods - these build and return the resource
 
     // TODO - would it be more convenient to make Registrar a singleton, like the incon manager?
     // Then it wouldn't have to be passed around just to give to these builders.
     // We already assume you have exactly one registrar, because building it initializes all the singletons...
-    public Resource<Discrete<T>> registered(Registrar registrar) {
+    public MutableResource<Discrete<T>> registered(Registrar registrar) {
       var result = notRegistered();
+      assertSet("name", name);
+      assertSet("value mapper", valueMapper);
       registrar.discrete(name, result, valueMapper);
       return result;
     }
 
-    public Resource<Discrete<T>> notRegistered() {
-      return resource(inconBehavior, effectTrait);
+    public MutableResource<Discrete<T>> notRegistered() {
+      // If no incon behavior is set, default to serializing the value in the default way.
+      if (inconBehavior == null) serialized();
+      assertSet("effect trait", effectTrait);
+      var result = resource(inconBehavior, effectTrait);
+      if (name != null) Naming.name(result, name);
+      return result;
     }
   }
 
-  // --- REWRITE START ---
-
-  // General discrete cell resource constructor
-  public static <T> MutableResource<Discrete<T>> discreteResource(T initialValue) {
-    return resource(discrete(initialValue));
-  }
-
-  // Annoyingly, we need to repeat the specialization for integer resources, so that
-  // discreteMutableResource(42) doesn't become a double resource, due to the next overload
-  public static MutableResource<Discrete<Integer>> discreteResource(int initialValue) {
-    return resource(discrete(initialValue));
-  }
-
-  // specialized constructor for doubles, because they require a toleranced equality comparison
-  public static MutableResource<Discrete<Double>> discreteResource(double initialValue) {
-    return resource(discrete(initialValue), autoEffects(testing(
-        (CommutativityTestInput<Discrete<Double>> input) -> DoubleUtils.areEqualResults(
-            input.original().extract(),
-            input.leftResult().extract(),
-            input.rightResult().extract()))));
-  }
-
-  // --- REWRITE END ---
-
   /**
    * Returns a condition that's satisfied whenever this resource is true.
    */
@@ -150,7 +197,9 @@ public static Condition when(Resource<Discrete<Boolean>> resource) {
    * Cache resource, updating the cache when updatePredicate(cached value, resource value) is true.
    */
   public static <V> Resource<Discrete<V>> cache(Resource<Discrete<V>> resource, BiPredicate<V, V> updatePredicate) {
-    final var cell = resource(resource.getDynamics());
+    // Caches are logically derived from the resource they're caching, so should not be directly saved.
+    // Instead, the resource(s) feeding this cache should be saved, and used to reinitialize this.
+    final var cell = resource(notSaving(resource.getDynamics()));
     // TODO: Does the update predicate need to propagate expiry information?
     BiPredicate<ErrorCatching<Expiring<Discrete<V>>>, ErrorCatching<Expiring<Discrete<V>>>> liftedUpdatePredicate = (eCurrent, eNew) ->
         eCurrent.match(
@@ -178,7 +227,7 @@ public static <V> Resource<Discrete<V>> cache(Resource<Discrete<V>> resource, Bi
    * Sample valueSupplier once every samplePeriod.
    */
   public static <V, T extends Dynamics<Duration, T>> Resource<Discrete<V>> sampled(Supplier<V> valueSupplier, Resource<T> samplePeriod) {
-    var result = discreteResource(valueSupplier.get());
+    var result = discreteResource(valueSupplier.get()).notSaved().notRegistered();
     every(() -> currentValue(samplePeriod, Duration.MAX_VALUE),
           () -> set(result, valueSupplier.get()));
     return result;
@@ -191,8 +240,7 @@ public static <V, T extends Dynamics<Duration, T>> Resource<Discrete<V>> sampled
    */
   public static <V> Resource<Discrete<V>> precomputed(
       final V valueBeforeFirstEntry, final NavigableMap<Duration, V> segments) {
-    var clock = clock();
-    return signalling(bind(clock, (Clock clock$) -> {
+    return signalling(bind(simulationClock(), (Clock clock$) -> {
       var t = clock$.extract();
       var entry = segments.floorEntry(t);
       var value = entry == null ? valueBeforeFirstEntry : entry.getValue();