Open
Conversation
…nd rescheduling, TemporalEventSourceDelta
Need to see if currentTime is a problem for tasks running in the past.
…ie into prototype/incremental-sim
The TimePoint.Deltas in TemporalEventSource make dealing with old and new timelines too awkward.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
.../test/java/gov/nasa/ammos/aerie/merlin/driver/test/property/IncrementalSimPropertyTests.java
Fixed
Show fixed
Hide fixed
bradNASA
commented
Aug 4, 2025
Contributor
Author
There was a problem hiding this comment.
SimulationResultsInterface was added so that the results of multiple SimulationEngines can be combined lazily.
bradNASA
commented
Aug 4, 2025
Contributor
Author
There was a problem hiding this comment.
This lets you exclude running specific tests with something like this:
./gradlew test -PexcludeTests="**/IntegrationTest*"
bradNASA
commented
Aug 4, 2025
bradNASA
commented
Aug 4, 2025
Contributor
Author
There was a problem hiding this comment.
It would be good to know why equals() and maybe hashCode() are necessary.
bradNASA
commented
Aug 4, 2025
Contributor
Author
There was a problem hiding this comment.
Added switch to optionally run newly added daemon tasks in the mission model since only some newly added tests need it.
Contributor
|
🫨 |
|
The counter 'i' is used to differentiate old and new TemporalEventSource instances during incremental simulation, helping track which event sources belong to which simulation engine in the chain.
- Created INCREMENTAL_SIM_PR_SUMMARY.md: Comprehensive analysis of all changes in the incremental simulation PR, organized by module - Added to .gitignore: docker-compose.override.yml, output*.json, report-tests.sh (local development/testing files)
Merged 136 commits from origin/develop to bring the incremental simulation branch up to date. Conflicts resolved: - orchestration-utils/.../SimulationResultsWriter.java: Updated to use SimulationResultsInterface with accessor methods (getRealProfiles(), getDiscreteProfiles(), etc.) - stateless-aerie/.../CLIArgumentsTest.java: Accepted develop's test assertion fix - stateless-aerie/.../simpleFooPlanResults.json: Accepted develop's test fixture with updated JSON structure Key changes from develop: - JUnit 5.10.0 → 6.0.1 upgrade - New @description and @subSYStem annotations for mission models - Workspace permissions and action-server improvements - Database migrations (27-30) - Various bug fixes and dependency updates 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Updated simpleFooPlanResults.json test fixture to match new JSON output format from develop - Field ordering changed in spans (directiveId, parentId, childIds now before type) - Activity and topics ordering changed - Removed extraneous blank line at start of file - Added .claude/settings.local.json to .gitignore for local-only settings - Added TEST_FAILURES_POST_MERGE.md documenting merge test results - stateless-aerie tests now pass (2 failures fixed) - merlin-driver-test has flaky tests (known issue, not caused by merge) Test results: - stateless-aerie: All tests passing ✓ - merlin-driver-test: 11 flaky test failures (pre-existing issue) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Added comprehensive documentation of merlin-driver-test flaky behavior: - Root cause analysis (race condition in TestContext static field) - Two attempted fixes and why they failed (ThreadLocal approach broke sharing) - Test failure patterns across multiple runs (0-13 failures, timing-dependent) - Recommendations for future fixes if needed Updated CLAUDE.md with documentation structure: - docs/investigations/ for technical writeups - .tmp/ directory for scratch work (already in .gitignore) Updated .gitignore to exclude local development files: - docker-compose override files - output*.json test artifacts - report-tests.sh helper script - .claude/settings.local.json 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Merged 136 commits from origin/develop into prototype/incremental-sim. This brings the prototype branch up to date with latest develop changes. Key changes: - Updated JUnit 5.10.0 → 6.0.1 - JSON serialization improvements (field ordering changes) - Database migrations 27-30 - New @description and @subSYStem annotations Test results after merge: - stateless-aerie: 2 failures FIXED (updated test fixtures) - merlin-driver-test: 0-1 flaky failures (pre-existing, not caused by merge) - All other modules: PASSING Flaky test investigation documented in: docs/investigations/flaky-tests-investigation-2025-12-30.md 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Implemented ThreadLocal context isolation with explicit context passing to eliminate race conditions in test execution. Problem: - TestContext was a static field shared across main and worker threads - Worker threads executed tasks asynchronously, accessing context - Race condition: worker could access context before it was set or after it was cleared, causing intermittent NullPointerExceptions - Tests showed 0-11 flaky failures depending on timing Solution: 1. Made TestContext use ThreadLocal for per-thread isolation 2. Added contextQueue to pass context from main thread to worker thread 3. Worker thread explicitly receives and sets context in its ThreadLocal 4. Context is updated on task resume (after delay/call/wait) Changes: - TestContext: Changed from static field to ThreadLocal, added setContext()/clearContext() for manual lifecycle management - ThreadedTask: Added contextQueue to TaskThread record, modified step() to pass context to worker, updated start() to receive and set context - Worker yield methods (delay/call/waitUntil): Update context on resume Results: - Before: 0-11 flaky failures (highly variable) - After: Consistently 1 failure (IncrementalSimPropertyTests, unrelated) - 10+ test runs show stable behavior with only property test failing The remaining IncrementalSimPropertyTests failure is a separate issue unrelated to TestContext race conditions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Extracted from jqwik property test (seed: -1474950772479154912).
test8: Minimal shrunk version showing activities deleted at T=3584
and topics not being unmarked when re-added.
test9: Full scenario demonstrating EventGraph.Concurrently grouping
differences between regular and incremental simulation.
At T=4, events that should be concurrent (39493 | 39493)
appear sequential (39493; 39493) in incremental sim.
These explicit test cases ensure the bug is reproducible in CI
regardless of jqwik seed randomization.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Implementation of in-memory incremental simulation. By default, this will be turned off except for scheduling.
Details can be found in presentations (that should be downloaded as pptx instead of viewing on web):
Also see the Incremental Simulation discussion.
Merging these changes avoids maintaining this as a separate version, which is problematic for architecture changes that do not consider this version. Updating this branch for new Aerie versions developed in parallel has been almost as much work as the implementation.
Verification
Tests added: IncrementalSimTests, EdgeCaseTests, GeneratedTests
The EdgeCaseTests and GeneratedTests tests validate against a past version of the develop branch copied as a module (merlin-driver-develop). Other implementations may also be compared; and alternative version of incremental sim is included).
Passes all tests
Documentation
No documentation has been invalidated. Documentation is needed.
Future work
Work needed before merging
Future work potentially beyond this pull request
SimulationEngines into one for performance/memory improvement.assertDirectiveNotRerun(ActivityDirectiveId)