diff --git a/.github/workflows/run_tests.yml b/.github/workflows/run_tests.yml index 2ec1e1b..78ebf40 100644 --- a/.github/workflows/run_tests.yml +++ b/.github/workflows/run_tests.yml @@ -62,3 +62,35 @@ jobs: hide-comment: false report-only-changed-files: false junitxml-path: ./pytest.xml + + # The pip job is pinned to Python 3.8 by pykep's wheels, but that is exactly the + # version where asyncio.get_event_loop() still papers over a missing loop, so the + # 3.10+ path would otherwise go untested. conda gets us a modern Python and + # doubles as a check that environment.yml still solves. + build-conda: + runs-on: ubuntu-latest + timeout-minutes: 30 + env: + MPLBACKEND: Agg + PYTHONUNBUFFERED: "1" + permissions: + contents: read + steps: + - uses: actions/checkout@v4 + - name: Set up conda environment from environment.yml + uses: conda-incubator/setup-miniconda@v3 + with: + environment-file: environment.yml + activate-environment: paseos + miniforge-version: latest + # environment.yml only says python>=3.8, so guard the reason this job exists: + # if conda ever resolves back to <3.10 the event-loop regression test stops + # covering anything and CI would stay green. + - name: Report resolved versions and assert Python 3.10+ + shell: bash -el {0} + run: | + python -c "import sys, pykep, numpy; print(sys.version); print(pykep.__version__, numpy.__version__)" + python -c "import sys; assert sys.version_info >= (3, 10), f'conda resolved {sys.version_info}, this job must run 3.10+'" + - name: Test with pytest + shell: bash -el {0} + run: pytest -v --timeout=180 paseos/tests/ diff --git a/paseos/activities/activity_manager.py b/paseos/activities/activity_manager.py index ce14d19..9c2f916 100644 --- a/paseos/activities/activity_manager.py +++ b/paseos/activities/activity_manager.py @@ -148,9 +148,20 @@ async def job(): # Workaround to avoid error when executed in a Jupyter notebook. self._paseos_instance._local_actor._current_activity = name - # Run activity and processor - loop = asyncio.get_event_loop() - if loop.is_running(): + # Run activity and processor. asyncio.run() refuses to nest inside an + # already-running loop (e.g. a Jupyter notebook), so ask whether one is + # running rather than using get_event_loop(): the latter raises once + # asyncio.run() has torn down and unset the loop, which made every + # perform_activity() after the first one fail on Python 3.10+. + try: + asyncio.get_running_loop() + loop_is_running = True + except RuntimeError: + loop_is_running = False + + # Deliberately outside the except block: running the activity inside the + # handler would chain any RuntimeError it raises onto "no running event loop". + if loop_is_running: asyncio.gather(job()) else: asyncio.run(job()) diff --git a/paseos/paseos.py b/paseos/paseos.py index b9e4fe1..75b1ad4 100644 --- a/paseos/paseos.py +++ b/paseos/paseos.py @@ -189,12 +189,14 @@ def advance_time( time_since_constraint_check += dt self.local_actor.set_time(pk.epoch(self._state.time * pk.SEC2DAY)) - # Check if we should update the status log - if self._time_since_previous_log > self._cfg.io.logging_interval: + # Check if we should update the status log. The elapsed time has to + # accumulate on every step, including the one that logs -- incrementing + # only in the else branch cost one extra step per interval, so a 10s + # interval at dt=10 logged every 30s. + self._time_since_previous_log += dt + if self._time_since_previous_log >= self._cfg.io.logging_interval: self.log_status() self._time_since_previous_log = 0 - else: - self._time_since_previous_log += dt logger.debug("New time is: " + str(self._state.time) + " s.") return max(target_time - self._state.time, 0) diff --git a/paseos/tests/activity_test.py b/paseos/tests/activity_test.py index acafa46..157515a 100644 --- a/paseos/tests/activity_test.py +++ b/paseos/tests/activity_test.py @@ -156,3 +156,26 @@ async def on_termination(args): assert test_value == test_value2 assert sat1.battery_level_in_Ws >= 505 + + +def test_perform_activity_twice_in_a_row(): + """Two consecutive perform_activity calls have to work. + + Deliberately a sync test: the async tests above run inside an event loop and so + take the asyncio.gather branch, which is why this survived. In the sync branch + asyncio.run() unsets the current event loop, so the old asyncio.get_event_loop() + lookup raised RuntimeError on the second call from Python 3.10 onwards. + """ + sim, _, _ = get_default_instance() + + results = [] + + async def func(args): + results.append(1) + + sim.register_activity("Testing", activity_function=func, power_consumption_in_watt=10) + + sim.perform_activity("Testing") + sim.perform_activity("Testing") + + assert len(results) == 2, "Both activity runs should have executed." diff --git a/paseos/tests/operations_monitor_test.py b/paseos/tests/operations_monitor_test.py index 2e4463c..264806a 100644 --- a/paseos/tests/operations_monitor_test.py +++ b/paseos/tests/operations_monitor_test.py @@ -54,3 +54,38 @@ async def func2(args): sim.monitor.plot("state_of_charge") sim.save_status_log_csv("test.csv") + + +@pytest.mark.parametrize( + "dt,logging_interval,expected", + [ + # Interval equal to the timestep: a log on every step. + (10.0, 10.0, [10, 20, 30, 40, 50, 60, 70, 80, 90, 100]), + # Interval spanning several steps. This is the case that distinguishes the + # fix from "log unconditionally": before it, the step that logged did not + # count towards the next interval, stretching every gap by one dt. + (10.0, 30.0, [10, 40, 70, 100]), + (1.0, 10.0, [1, 11, 21, 31, 41, 51, 61, 71, 81, 91]), + ], +) +def test_logging_interval_is_respected(dt, logging_interval, expected): + """The status log has to fire at the configured interval, not a multiple of it.""" + earth = pk.planet.jpl_lp("earth") + sat1 = ActorBuilder.get_actor_scaffold("sat1", SpacecraftActor, pk.epoch(0)) + ActorBuilder.set_orbit(sat1, [10000000, 0, 0], [0, 8000.0, 0], pk.epoch(0), earth) + ActorBuilder.set_power_devices(sat1, 500, 10000, 1) + + cfg = load_default_cfg() + cfg.sim.start_time = 0.0 + cfg.sim.dt = dt + cfg.io.logging_interval = logging_interval + sim = paseos.init_sim(sat1, cfg) + + sim.advance_time(100.0, 0) + + # Compared with a tolerance: the simulation time accumulates dt step by step, + # so exact equality would trip over float representation. + timesteps = sim.monitor["timesteps"] + assert timesteps == pytest.approx(expected), ( + f"With dt={dt} and interval={logging_interval} expected logs at {expected}, got {timesteps}" + ) diff --git a/pyproject.toml b/pyproject.toml index f9506fd..5472ece 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -8,7 +8,10 @@ version = "0.2.1" description = "A package which simulates the space environment for operating multiple spacecraft." readme = "README.md" license = { file = "LICENSE" } -requires-python = ">=3.8" +# Upper bound follows the hard numpy==1.23.5 pin below: numpy 1.23.5 has no +# wheels for 3.12+ and its sdist does not build there, so an uncapped +# requires-python advertises installs that cannot possibly succeed. +requires-python = ">=3.8,<3.12" authors = [{ name = "Φ-lab@Sweden", email = "pablo.gomez@esa.int" }] keywords = ["spacecraft", "simulation", "space", "satellite", "constellation"] classifiers = [