From faa7c03308e65ccb4881e3150e09ce2d5fe6d8d9 Mon Sep 17 00:00:00 2001 From: JoePfeiffer Date: Sat, 23 Sep 2023 14:19:59 -0600 Subject: [PATCH 1/6] Make store persistent instead of creating a new one every step() Use timestep remaining in store instead of getPreviousTimeStep() and setPreviousTimeStep() --- .../simulation/RK4SimulationStepper.java | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/core/src/net/sf/openrocket/simulation/RK4SimulationStepper.java b/core/src/net/sf/openrocket/simulation/RK4SimulationStepper.java index 16cd9fd35..2801fbc6a 100644 --- a/core/src/net/sf/openrocket/simulation/RK4SimulationStepper.java +++ b/core/src/net/sf/openrocket/simulation/RK4SimulationStepper.java @@ -59,9 +59,7 @@ public class RK4SimulationStepper extends AbstractSimulationStepper { private static final double MAX_PITCH_CHANGE = 4 * Math.PI / 180; private Random random; - - - + DataStore store = new DataStore(); @Override public RK4SimulationStatus initialize(SimulationStatus original) { @@ -90,28 +88,26 @@ public class RK4SimulationStepper extends AbstractSimulationStepper { public void step(SimulationStatus simulationStatus, double maxTimeStep) throws SimulationException { RK4SimulationStatus status = (RK4SimulationStatus) simulationStatus; - DataStore store = new DataStore(); //////// Perform RK4 integration: //////// RK4SimulationStatus status2; RK4Parameters k1, k2, k3, k4; + /* * Start with previous time step which is used to compute the initial thrust estimate. * Don't make it longer than maxTimeStep, but at least MIN_TIME_STEP. */ - store.timestep = status.getPreviousTimeStep(); store.timestep = MathUtil.max(MathUtil.min(store.timestep, maxTimeStep), MIN_TIME_STEP); checkNaN(store.timestep); - + /* * Compute the initial thrust estimate. This is used for the first time step computation. */ - store.thrustForce = calculateAverageThrust(status, store.timestep, status.getPreviousAcceleration(), + store.thrustForce = calculateAverageThrust(status, store.timestep, store.timestep, status.getPreviousAtmosphericConditions(), false); - /* * Perform RK4 integration. Decide the time step length after the first step. */ @@ -150,7 +146,7 @@ public class RK4SimulationStepper extends AbstractSimulationStepper { dt[0] /= 5.0; dt[6] = status.getSimulationConditions().getLaunchRodLength() / k1.v.length() / 10; } - dt[7] = 1.5 * status.getPreviousTimeStep(); + dt[7] = 1.5 * store.timestep; store.timestep = Double.MAX_VALUE; int limitingValue = -1; @@ -261,8 +257,6 @@ public class RK4SimulationStepper extends AbstractSimulationStepper { } status.setSimulationTime(status.getSimulationTime() + store.timestep); - status.setPreviousTimeStep(store.timestep); - // Store data // TODO: MEDIUM: Store acceleration etc of entire RK4 step, store should be cloned or something... storeData(status, store); From 1b29a6b767f8c8882120410865e142b857f2fd1c Mon Sep 17 00:00:00 2001 From: JoePfeiffer Date: Sat, 23 Sep 2023 14:23:59 -0600 Subject: [PATCH 2/6] Testing time on events that are determined by simulation steppers shouldn't be more precise than the simulation time step. --- .../sf/openrocket/simulation/FlightEventsTest.java | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/core/test/net/sf/openrocket/simulation/FlightEventsTest.java b/core/test/net/sf/openrocket/simulation/FlightEventsTest.java index fe80e83c8..9830c3ce6 100644 --- a/core/test/net/sf/openrocket/simulation/FlightEventsTest.java +++ b/core/test/net/sf/openrocket/simulation/FlightEventsTest.java @@ -67,8 +67,12 @@ public class FlightEventsTest extends BaseTestCase { // Test that the event times are correct for (int i = 0; i < expectedEventTimes.length; i++) { + FlightEvent actual = eventList.get(i); + double epsilon = ((actual.getType() == FlightEvent.Type.TUMBLE) || + (actual.getType() == FlightEvent.Type.APOGEE)) ? sim.getOptions().getTimeStep() : EPSILON; + assertEquals(" Flight type " + expectedEventTypes[i] + " has wrong time ", - expectedEventTimes[i], eventList.get(i).getTime(), EPSILON); + expectedEventTimes[i], actual.getTime(), epsilon); } @@ -169,8 +173,10 @@ public class FlightEventsTest extends BaseTestCase { expected.getType(), actual.getType()); if (1200 != expected.getTime()) { - // Tumbling can have a very large time error, so implement a more course epsilon (otherwise unit tests just keep failing...) - double epsilon = actual.getType() == FlightEvent.Type.TUMBLE ? 0.05 : EPSILON; + // Events whose timing depends on the results of the steppers can't be expected to be more accurate than + // the length of a time step + double epsilon = ((actual.getType() == FlightEvent.Type.TUMBLE) || + (actual.getType() == FlightEvent.Type.APOGEE)) ? sim.getOptions().getTimeStep() : EPSILON; assertEquals("Branch " + b + " FlightEvent " + i + " type " + expected.getType() + " has wrong time ", expected.getTime(), actual.getTime(), epsilon); } From 65b36af180c9056fc7445fbd41540d933ad0e22a Mon Sep 17 00:00:00 2001 From: JoePfeiffer Date: Sun, 24 Sep 2023 16:37:43 -0600 Subject: [PATCH 3/6] cleaned up FlightEventsTest by (1) creating a CompareEvents() method called by both testSingleStage() and testMultiStage() to get rid of duplicate code; also made creation of expected events in testSingleStage() cleaner (2) tied required time accuracy of events that depend on one of the simulationsteppers to the simulation timestep. This also let us go back to testing time on all events. --- .../simulation/FlightEventsTest.java | 119 ++++++++---------- 1 file changed, 53 insertions(+), 66 deletions(-) diff --git a/core/test/net/sf/openrocket/simulation/FlightEventsTest.java b/core/test/net/sf/openrocket/simulation/FlightEventsTest.java index 9830c3ce6..a6fae3e22 100644 --- a/core/test/net/sf/openrocket/simulation/FlightEventsTest.java +++ b/core/test/net/sf/openrocket/simulation/FlightEventsTest.java @@ -43,44 +43,24 @@ public class FlightEventsTest extends BaseTestCase { final int branchCount = sim.getSimulatedData().getBranchCount(); assertEquals(" Single stage simulation invalid branch count ", 1, branchCount); - final FlightEvent.Type[] expectedEventTypes = {FlightEvent.Type.LAUNCH, FlightEvent.Type.IGNITION, FlightEvent.Type.LIFTOFF, - FlightEvent.Type.LAUNCHROD, FlightEvent.Type.BURNOUT, FlightEvent.Type.EJECTION_CHARGE, FlightEvent.Type.RECOVERY_DEVICE_DEPLOYMENT, - FlightEvent.Type.APOGEE, FlightEvent.Type.GROUND_HIT, FlightEvent.Type.SIMULATION_END}; - final double[] expectedEventTimes = {0.0, 0.0, 0.1275, 0.13, 2.0, 2.0, 2.001, 2.48338}; // Ground hit time is too variable, so don't include it final AxialStage stage = rocket.getStage(0); final InnerTube motorMountTube = (InnerTube) stage.getChild(1).getChild(2); final Parachute parachute = (Parachute) stage.getChild(1).getChild(3); - final RocketComponent[] expectedSources = {rocket, motorMountTube, null, null, motorMountTube, - stage, parachute, rocket, null, null}; + + FlightEvent[] expectedEvents = new FlightEvent[] { + new FlightEvent(FlightEvent.Type.LAUNCH, 0.0, rocket), + new FlightEvent(FlightEvent.Type.IGNITION, 0.0, motorMountTube), + new FlightEvent(FlightEvent.Type.LIFTOFF, 0.1275, null), + new FlightEvent(FlightEvent.Type.LAUNCHROD, 0.13, null), + new FlightEvent(FlightEvent.Type.BURNOUT, 2.0, motorMountTube), + new FlightEvent(FlightEvent.Type.EJECTION_CHARGE, 2.0, stage), + new FlightEvent(FlightEvent.Type.RECOVERY_DEVICE_DEPLOYMENT, 2.001, parachute), + new FlightEvent(FlightEvent.Type.APOGEE, 2.48338, rocket), + new FlightEvent(FlightEvent.Type.GROUND_HIT, 43.1, null), + new FlightEvent(FlightEvent.Type.SIMULATION_END, 43.1, null) + }; - // Test event count - FlightDataBranch branch = sim.getSimulatedData().getBranch(0); - List eventList = branch.getEvents(); - List eventTypes = eventList.stream().map(FlightEvent::getType).collect(Collectors.toList()); - assertEquals(" Single stage simulation invalid number of events ", expectedEventTypes.length, eventTypes.size()); - - // Test that all expected events are present, and in the right order - for (int i = 0; i < expectedEventTypes.length; i++) { - assertSame(" Flight type " + expectedEventTypes[i] + " not found in single stage simulation ", - expectedEventTypes[i], eventTypes.get(i)); - } - - // Test that the event times are correct - for (int i = 0; i < expectedEventTimes.length; i++) { - FlightEvent actual = eventList.get(i); - double epsilon = ((actual.getType() == FlightEvent.Type.TUMBLE) || - (actual.getType() == FlightEvent.Type.APOGEE)) ? sim.getOptions().getTimeStep() : EPSILON; - - assertEquals(" Flight type " + expectedEventTypes[i] + " has wrong time ", - expectedEventTimes[i], actual.getTime(), epsilon); - - } - - // Test that the event sources are correct - for (int i = 0; i < expectedSources.length; i++) { - assertEquals(" Flight type " + expectedEventTypes[i] + " has wrong source ", - expectedSources[i], eventList.get(i).getSource()); - } + compareEvents(sim.getSimulatedData().getBranch(0), expectedEvents, sim.getOptions().getTimeStep()); } /** @@ -109,7 +89,6 @@ public class FlightEventsTest extends BaseTestCase { final InnerTube boosterMotorTubes = (InnerTube) boosterStage.getChild(1).getChild(0); final BodyTube coreBody = (BodyTube) coreStage.getChild(0); - // events whose time is too variable to check are given a time of 1200 for (int b = 0; b < 3; b++) { FlightEvent[] expectedEvents; switch (b) { @@ -129,8 +108,8 @@ public class FlightEventsTest extends BaseTestCase { new FlightEvent(FlightEvent.Type.EJECTION_CHARGE, 2.0, coreStage), new FlightEvent(FlightEvent.Type.STAGE_SEPARATION, 2.0, coreStage), new FlightEvent(FlightEvent.Type.TUMBLE, 2.4127, null), - new FlightEvent(FlightEvent.Type.GROUND_HIT, 1200, null), - new FlightEvent(FlightEvent.Type.SIMULATION_END, 1200, null) + new FlightEvent(FlightEvent.Type.GROUND_HIT, 13.2, null), + new FlightEvent(FlightEvent.Type.SIMULATION_END, 13.2, null) }; break; // Core stage @@ -140,8 +119,8 @@ public class FlightEventsTest extends BaseTestCase { new FlightEvent(FlightEvent.Type.BURNOUT, 2.0, coreBody), new FlightEvent(FlightEvent.Type.EJECTION_CHARGE, 2.0, coreStage), new FlightEvent(FlightEvent.Type.STAGE_SEPARATION, 2.0, coreStage), - new FlightEvent(FlightEvent.Type.GROUND_HIT, 1200, null), - new FlightEvent(FlightEvent.Type.SIMULATION_END, 1200, null) + new FlightEvent(FlightEvent.Type.GROUND_HIT, 5.9, null), + new FlightEvent(FlightEvent.Type.SIMULATION_END, 5.9, null) }; break; // Booster stage @@ -152,39 +131,47 @@ public class FlightEventsTest extends BaseTestCase { new FlightEvent(FlightEvent.Type.EJECTION_CHARGE, 2.0, boosterStage), new FlightEvent(FlightEvent.Type.STAGE_SEPARATION, 2.0, boosterStage), new FlightEvent(FlightEvent.Type.TUMBLE, 3.428, null), - new FlightEvent(FlightEvent.Type.GROUND_HIT, 1200, null), - new FlightEvent(FlightEvent.Type.SIMULATION_END, 1200, null) + new FlightEvent(FlightEvent.Type.GROUND_HIT, 10.32, null), + new FlightEvent(FlightEvent.Type.SIMULATION_END, 10.32, null) }; break; default: throw new IllegalStateException("Invalid branch number " + b); } - // Test event count - final FlightDataBranch branch = sim.getSimulatedData().getBranch(b); - final FlightEvent[] events = branch.getEvents().toArray(new FlightEvent[0]); - assertEquals(" Multi-stage simulation, branch " + b + " invalid number of events ", expectedEvents.length, events.length); - - // Test that all expected events are present, in the right order, at the right time, from the right sources - for (int i = 0; i < events.length; i++) { - final FlightEvent expected = expectedEvents[i]; - final FlightEvent actual = events[i]; - assertSame("Branch " + b + " FlightEvent " + i + " type " + expected.getType() + " not found; FlightEvent " + actual.getType() + " found instead", - expected.getType(), actual.getType()); - - if (1200 != expected.getTime()) { - // Events whose timing depends on the results of the steppers can't be expected to be more accurate than - // the length of a time step - double epsilon = ((actual.getType() == FlightEvent.Type.TUMBLE) || - (actual.getType() == FlightEvent.Type.APOGEE)) ? sim.getOptions().getTimeStep() : EPSILON; - assertEquals("Branch " + b + " FlightEvent " + i + " type " + expected.getType() + " has wrong time ", - expected.getTime(), actual.getTime(), epsilon); - } - - // Test that the event sources are correct - assertEquals("Branch " + b + " FlightEvent " + i + " type " + expected.getType() + " has wrong source ", - expected.getSource(), actual.getSource()); - } + compareEvents(sim.getSimulatedData().getBranch(b), expectedEvents, sim.getOptions().getTimeStep()); } } + + // compare expected vs. actual event types, times, and sources + // Note: if an event's time is too variable to test reliably, set expected time to 1200 to ignore + private void compareEvents(FlightDataBranch b, FlightEvent[] expectedEvents, double timeStep) { + FlightEvent[] actualEvents = b.getEvents().toArray(new FlightEvent[0]); + + // test event count + assertEquals("Number of flight events in branch " + b, expectedEvents.length, actualEvents.length); + + // Test that all expected events are present, in the right order, at the right time, from the right sources + for (int i = 0; i < actualEvents.length; i++) { + final FlightEvent expected = expectedEvents[i]; + final FlightEvent actual = actualEvents[i]; + System.out.println("event " + actual); + assertSame("Branch " + b + " FlightEvent " + i + " type " + expected.getType() + " not found; FlightEvent " + actual.getType() + " found instead", + expected.getType(), actual.getType()); + + if (1200 != expected.getTime()) { + // Events whose timing depends on the results of the steppers can't be expected to be more accurate than + // the length of a time step + double epsilon = ((actual.getType() == FlightEvent.Type.TUMBLE) || + (actual.getType() == FlightEvent.Type.APOGEE) || + (actual.getType() == FlightEvent.Type.GROUND_HIT) || + (actual.getType() == FlightEvent.Type.SIMULATION_END)) ? timeStep : EPSILON; + assertEquals("Branch " + b + " FlightEvent " + i + " type " + expected.getType() + " has wrong time ", + expected.getTime(), actual.getTime(), epsilon); + } + + assertEquals("FlightEvent from unexpected source", expected.getSource(), actual.getSource()); + } + } + } From 7a2ede6e2491740b41836debf3e10c99e06b4b6e Mon Sep 17 00:00:00 2001 From: JoePfeiffer Date: Mon, 25 Sep 2023 18:43:05 -0600 Subject: [PATCH 4/6] Use current atmospheric conditions, not previous conditions, in calculating thrust. This lets us get rid of previousAtmosphericConditions completely. Note that while we pass acceleration and atmospheric conditions into calculateAverageThrust(), we don't actually do anything with them there. They appear to be passed in for some sort of eventual enhancements when we do things like altitude compensation for motors, or acceleration for hybrids or water rockets. It also turned out that while we called getPreviousAtmosphericConditions() when call ing calculateAverageThrust(), setPreviousAtmosphericConditions() was never called. --- .../openrocket/simulation/RK4SimulationStatus.java | 14 -------------- .../simulation/RK4SimulationStepper.java | 10 ++++++++-- 2 files changed, 8 insertions(+), 16 deletions(-) diff --git a/core/src/net/sf/openrocket/simulation/RK4SimulationStatus.java b/core/src/net/sf/openrocket/simulation/RK4SimulationStatus.java index 67465e16d..d42d951fa 100644 --- a/core/src/net/sf/openrocket/simulation/RK4SimulationStatus.java +++ b/core/src/net/sf/openrocket/simulation/RK4SimulationStatus.java @@ -8,7 +8,6 @@ public class RK4SimulationStatus extends SimulationStatus implements Cloneable { private Coordinate launchRodDirection; private double previousAcceleration = 0; - private AtmosphericConditions previousAtmosphericConditions; // Used for determining when to store aerodynamic computation warnings: private double maxZVelocity = 0; @@ -26,7 +25,6 @@ public class RK4SimulationStatus extends SimulationStatus implements Cloneable { this.previousAcceleration = ((RK4SimulationStatus) other).previousAcceleration; this.maxZVelocity = ((RK4SimulationStatus) other).maxZVelocity; this.startWarningTime = ((RK4SimulationStatus) other).startWarningTime; - this.previousAtmosphericConditions = ((RK4SimulationStatus) other).previousAtmosphericConditions; } } public void setLaunchRodDirection(Coordinate launchRodDirection) { @@ -49,18 +47,6 @@ public class RK4SimulationStatus extends SimulationStatus implements Cloneable { this.previousAcceleration = previousAcceleration; } - - public AtmosphericConditions getPreviousAtmosphericConditions() { - return previousAtmosphericConditions; - } - - - public void setPreviousAtmosphericConditions( - AtmosphericConditions previousAtmosphericConditions) { - this.previousAtmosphericConditions = previousAtmosphericConditions; - } - - public double getMaxZVelocity() { return maxZVelocity; } diff --git a/core/src/net/sf/openrocket/simulation/RK4SimulationStepper.java b/core/src/net/sf/openrocket/simulation/RK4SimulationStepper.java index 2801fbc6a..9c1b825af 100644 --- a/core/src/net/sf/openrocket/simulation/RK4SimulationStepper.java +++ b/core/src/net/sf/openrocket/simulation/RK4SimulationStepper.java @@ -102,11 +102,17 @@ public class RK4SimulationStepper extends AbstractSimulationStepper { store.timestep = MathUtil.max(MathUtil.min(store.timestep, maxTimeStep), MIN_TIME_STEP); checkNaN(store.timestep); + /* + * Get the current atmospheric conditions + */ + calculateFlightConditions(status, store); + store.atmosphericConditions = store.flightConditions.getAtmosphericConditions(); + /* * Compute the initial thrust estimate. This is used for the first time step computation. */ - store.thrustForce = calculateAverageThrust(status, store.timestep, store.timestep, - status.getPreviousAtmosphericConditions(), false); + store.thrustForce = calculateAverageThrust(status, store.timestep, store.longitudinalAcceleration, + store.atmosphericConditions, false); /* * Perform RK4 integration. Decide the time step length after the first step. From 7bafe94f23169ab2a451af80cea529df07c7a167 Mon Sep 17 00:00:00 2001 From: JoePfeiffer Date: Tue, 26 Sep 2023 16:48:54 -0600 Subject: [PATCH 5/6] Pass acceleration data to post listeners Also, existing code wouldn't call calculation post-listeners if pre-listeners had been called. Fixes that. --- .../simulation/RK4SimulationStepper.java | 27 +++++++++---------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/core/src/net/sf/openrocket/simulation/RK4SimulationStepper.java b/core/src/net/sf/openrocket/simulation/RK4SimulationStepper.java index 9c1b825af..1e24d0924 100644 --- a/core/src/net/sf/openrocket/simulation/RK4SimulationStepper.java +++ b/core/src/net/sf/openrocket/simulation/RK4SimulationStepper.java @@ -283,11 +283,17 @@ public class RK4SimulationStepper extends AbstractSimulationStepper { throws SimulationException { RK4Parameters params = new RK4Parameters(); - // if (dataStore == null) { - // dataStore = new DataStore(); - // } + // Call pre-listeners + store.accelerationData = SimulationListenerHelper.firePreAccelerationCalculation(status); + + // Calculate acceleration (if not overridden by pre-listeners) + if (store.accelerationData == null) { + store.accelerationData = calculateAcceleration(status, dataStore); + } + + // Call post-listeners + store.accelerationData = SimulationListenerHelper.firePostAccelerationCalculation(status, store.accelerationData); - calculateAcceleration(status, dataStore); params.a = dataStore.linearAcceleration; params.ra = dataStore.angularAcceleration; params.v = status.getRocketVelocity(); @@ -312,13 +318,7 @@ public class RK4SimulationStepper extends AbstractSimulationStepper { * @param status the status of the rocket. * @throws SimulationException */ - private void calculateAcceleration(RK4SimulationStatus status, DataStore store) throws SimulationException { - - // Call pre-listeners - store.accelerationData = SimulationListenerHelper.firePreAccelerationCalculation(status); - if (store.accelerationData != null) { - return; - } + private AccelerationData calculateAcceleration(RK4SimulationStatus status, DataStore store) throws SimulationException { // Compute the forces affecting the rocket calculateForces(status, store); @@ -403,9 +403,8 @@ public class RK4SimulationStepper extends AbstractSimulationStepper { store.angularAcceleration = status.getRocketOrientationQuaternion().rotate(store.angularAcceleration); } - - // Call post-listeners - store.accelerationData = SimulationListenerHelper.firePostAccelerationCalculation(status, store.accelerationData); + + return new AccelerationData(null, null, store.linearAcceleration, store.angularAcceleration, status.getRocketOrientationQuaternion()); } From b3dbedce4088c0276eef10fa8857469fe11f3a8d Mon Sep 17 00:00:00 2001 From: JoePfeiffer Date: Tue, 26 Sep 2023 17:16:14 -0600 Subject: [PATCH 6/6] get updated component file --- swing/resources-src/datafiles/components-dbcook | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/swing/resources-src/datafiles/components-dbcook b/swing/resources-src/datafiles/components-dbcook index 9da5f4e25..f4e30ecd1 160000 --- a/swing/resources-src/datafiles/components-dbcook +++ b/swing/resources-src/datafiles/components-dbcook @@ -1 +1 @@ -Subproject commit 9da5f4e25f57e8ec476ea38cd91c78edff06003f +Subproject commit f4e30ecd12c64803dce013c615bbe62f89785408