NASA-AMMOS / aerie

A software framework for modeling spacecraft.
https://nasa-ammos.github.io/aerie-docs/
MIT License
73 stars 19 forks source link

Simulation result staleness checking #1595

Closed JoelCourtney closed 1 week ago

JoelCourtney commented 2 weeks ago

Description

This PR implements a staleness check for the internal procedural scheduling implementations. The editable plan instance keeps track of sim results that it has produced using weak references, and can dynamically update their staleness if the plan is changed after it was simulated. The process is this:

  1. InMemoryEditablePlan has a set of weak references to simulation results objects that are currently up-to-date. I used weak references because if the user can't access it anymore, staleness doesn't matter and we might as well let it get gc'ed.
  2. When the user gets simulation results, either through simulation or by getting the latest, it always checks for plan equality between the returned results and the current plan, even if we just simulated. If it is up-to-date, a weak ref is added to the set.
  3. When an edit is made, the sim results in the current set are marked stale; then the set is reset to new reference to an empty set.
  4. When a commit is made, the commit object takes shared ownership of the set. If a new simulation is run (step 2) the plan can still add to the set while it is still jointly owned by the commit. Then when an edit is made (step 3) the commit will become the sole owner of the set.
  5. When changes are rolled back, any sim results currently in the plan's set are marked stale, the previous commit's sim results are marked not stale, then the plan will resume joint ownership of the previous commit's set.

The joint ownership freaks me out a wee bit, but I think it's safe because the commits are only used to keep the previous sets from getting gc'ed in the event of a rollback. Only the plan object actually mutates the set.

Verification

I added some unit tests to the scheduler-driver.

Documentation

This doesn't need to be explained to the user, but I've copied the above description into the doc comment on InMemoryEditablePlan.

Future work

JoelCourtney commented 1 week ago

Does moving Commit inside of the InMemoryEditablePlan help ease any anxieties about the shared ownership?

Nope. I just did it because the Commit class isn't used anywhere else, so I didn't want to make it look like it was part of a public interface.