From cb284590c402c7bf3569a7d61d9d99ec9b23be59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20G=C3=B3mez?= Date: Sat, 22 Aug 2026 13:08:26 +0200 Subject: [PATCH 1/2] fix: repeated activities and logging interval Found while validating the 0.2.1 artifact from TestPyPI. Both predate 0.2.0; neither is a regression from this release. - perform_activity() only worked once per process on Python 3.10+. asyncio.run() unsets the current event loop, so the next call's asyncio.get_event_loop() raised RuntimeError instead of silently creating a new loop as it did on 3.8/3.9. Ask get_running_loop() whether we are already inside a loop instead. - The status log fired every third step at dt == logging_interval: the elapsed-time counter was only incremented in the else branch, so the logging step itself did not count, and the comparison was strict. Also caps requires-python at <3.12, which the hard numpy==1.23.5 pin has always implied -- numpy 1.23.5 has no cp312 wheels and its sdist does not build there, so pip install could never succeed on 3.12+ on any platform. CI gains a conda job on the Python that environment.yml resolves to. The pip job is pinned to 3.8 by pykep's wheels, which is precisely the version where the event-loop bug does not reproduce, so the fix would otherwise have no coverage. --- .github/workflows/run_tests.yml | 27 +++++++++++++ environment.yml | 1 + paseos/activities/activity_manager.py | 15 ++++--- paseos/paseos.py | 10 +++-- paseos/tests/release_regression_test.py | 54 +++++++++++++++++++++++++ pyproject.toml | 7 +++- 6 files changed, 104 insertions(+), 10 deletions(-) create mode 100644 paseos/tests/release_regression_test.py diff --git a/.github/workflows/run_tests.yml b/.github/workflows/run_tests.yml index 2ec1e1b..957686a 100644 --- a/.github/workflows/run_tests.yml +++ b/.github/workflows/run_tests.yml @@ -62,3 +62,30 @@ 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 + - name: Report resolved versions + shell: bash -el {0} + run: python -c "import sys, pykep, numpy; print(sys.version); print(pykep.__version__, numpy.__version__)" + - name: Test with pytest + shell: bash -el {0} + run: pytest -v --timeout=180 paseos/tests/ diff --git a/environment.yml b/environment.yml index 18ec8a0..76f3cb1 100644 --- a/environment.yml +++ b/environment.yml @@ -11,6 +11,7 @@ dependencies: - pyquaternion>=0.9.9 # core non-optional dependency - pytest # for tests - pytest-asyncio # for tests involving activities + - pytest-timeout # tests are run with --timeout to catch hangs - pytest-cov # for coverage reports in CI-equivalent test runs - pytest-timeout # tests are run with --timeout to catch hangs - python>=3.8 # core non-optional dependency diff --git a/paseos/activities/activity_manager.py b/paseos/activities/activity_manager.py index ce14d19..9cc6ba4 100644 --- a/paseos/activities/activity_manager.py +++ b/paseos/activities/activity_manager.py @@ -148,11 +148,16 @@ 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(): - asyncio.gather(job()) - else: + # 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() + except RuntimeError: asyncio.run(job()) + else: + asyncio.gather(job()) logger.info(f"Activity {activity} completed.") 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/release_regression_test.py b/paseos/tests/release_regression_test.py new file mode 100644 index 0000000..f5496c8 --- /dev/null +++ b/paseos/tests/release_regression_test.py @@ -0,0 +1,54 @@ +"""Regression tests for bugs found while validating the 0.2.1 release.""" + +import pykep as pk +import pytest +from test_utils import get_default_instance + +import paseos +from paseos import ActorBuilder, SpacecraftActor, load_default_cfg + + +def test_perform_activity_twice_in_a_row(): + """Two consecutive perform_activity calls have to work. + + asyncio.run() unsets the current event loop, so the previous + asyncio.get_event_loop() lookup raised RuntimeError on the second call + from Python 3.10 onwards. Only reproduces on 3.10+, hence the conda CI job. + """ + 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." + + +def test_logging_interval_is_respected(): + """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 = 10.0 + cfg.io.logging_interval = 10.0 + 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. + # Before the fix this logged at 10, 40, 70, 100 -- every 30s. + timesteps = sim.monitor["timesteps"] + assert timesteps == pytest.approx([10, 20, 30, 40, 50, 60, 70, 80, 90, 100]), ( + f"Expected a log every 10s but got {timesteps}" + ) diff --git a/pyproject.toml b/pyproject.toml index f9506fd..8d80ec3 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 = [ @@ -19,6 +22,8 @@ classifiers = [ "License :: OSI Approved :: GNU General Public License v3 (GPLv3)", "Programming Language :: Python :: 3.8", "Programming Language :: Python :: 3.9", + "Programming Language :: Python :: 3.10", + "Programming Language :: Python :: 3.11", ] dependencies = [ "dotmap>=1.3.30", From 5e155531c31272e8828db0d54b619b25710ff47f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20G=C3=B3mez?= Date: Sat, 22 Aug 2026 13:21:20 +0200 Subject: [PATCH 2/2] fix: address review on logging oracle, loop detection and metadata - Parametrize the logging test over (dt, logging_interval). The single interval == dt case it had could not tell the fix apart from logging unconditionally on every step; (10, 30) can. - Move both tests into the feature modules the repo organises by (activity_test, operations_monitor_test) instead of a bucket named after when the bugs were found. - Assert Python 3.10+ in the conda job. environment.yml only asks for >=3.8, so without this the job could silently stop covering the version the event-loop fix is about. - Hoist the running-loop detection out of the except handler so a RuntimeError from user activity code is not chained onto 'no running event loop'. - Drop the duplicate pytest-timeout; #226 already added it. - Drop the 3.10/3.11 classifiers. requires-python and classifiers describe the pip path, where pykep's 2.x wheels stop at 3.8, so advertising them is the same inconsistency the <3.12 cap removes. --- .github/workflows/run_tests.yml | 9 ++++- environment.yml | 1 - paseos/activities/activity_manager.py | 10 ++++- paseos/tests/activity_test.py | 23 +++++++++++ paseos/tests/operations_monitor_test.py | 35 ++++++++++++++++ paseos/tests/release_regression_test.py | 54 ------------------------- pyproject.toml | 2 - 7 files changed, 73 insertions(+), 61 deletions(-) delete mode 100644 paseos/tests/release_regression_test.py diff --git a/.github/workflows/run_tests.yml b/.github/workflows/run_tests.yml index 957686a..78ebf40 100644 --- a/.github/workflows/run_tests.yml +++ b/.github/workflows/run_tests.yml @@ -83,9 +83,14 @@ jobs: environment-file: environment.yml activate-environment: paseos miniforge-version: latest - - name: Report resolved versions + # 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__)" + 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/environment.yml b/environment.yml index 76f3cb1..18ec8a0 100644 --- a/environment.yml +++ b/environment.yml @@ -11,7 +11,6 @@ dependencies: - pyquaternion>=0.9.9 # core non-optional dependency - pytest # for tests - pytest-asyncio # for tests involving activities - - pytest-timeout # tests are run with --timeout to catch hangs - pytest-cov # for coverage reports in CI-equivalent test runs - pytest-timeout # tests are run with --timeout to catch hangs - python>=3.8 # core non-optional dependency diff --git a/paseos/activities/activity_manager.py b/paseos/activities/activity_manager.py index 9cc6ba4..9c2f916 100644 --- a/paseos/activities/activity_manager.py +++ b/paseos/activities/activity_manager.py @@ -155,9 +155,15 @@ async def job(): # perform_activity() after the first one fail on Python 3.10+. try: asyncio.get_running_loop() + loop_is_running = True except RuntimeError: - asyncio.run(job()) - else: + 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()) logger.info(f"Activity {activity} completed.") 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/paseos/tests/release_regression_test.py b/paseos/tests/release_regression_test.py deleted file mode 100644 index f5496c8..0000000 --- a/paseos/tests/release_regression_test.py +++ /dev/null @@ -1,54 +0,0 @@ -"""Regression tests for bugs found while validating the 0.2.1 release.""" - -import pykep as pk -import pytest -from test_utils import get_default_instance - -import paseos -from paseos import ActorBuilder, SpacecraftActor, load_default_cfg - - -def test_perform_activity_twice_in_a_row(): - """Two consecutive perform_activity calls have to work. - - asyncio.run() unsets the current event loop, so the previous - asyncio.get_event_loop() lookup raised RuntimeError on the second call - from Python 3.10 onwards. Only reproduces on 3.10+, hence the conda CI job. - """ - 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." - - -def test_logging_interval_is_respected(): - """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 = 10.0 - cfg.io.logging_interval = 10.0 - 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. - # Before the fix this logged at 10, 40, 70, 100 -- every 30s. - timesteps = sim.monitor["timesteps"] - assert timesteps == pytest.approx([10, 20, 30, 40, 50, 60, 70, 80, 90, 100]), ( - f"Expected a log every 10s but got {timesteps}" - ) diff --git a/pyproject.toml b/pyproject.toml index 8d80ec3..5472ece 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -22,8 +22,6 @@ classifiers = [ "License :: OSI Approved :: GNU General Public License v3 (GPLv3)", "Programming Language :: Python :: 3.8", "Programming Language :: Python :: 3.9", - "Programming Language :: Python :: 3.10", - "Programming Language :: Python :: 3.11", ] dependencies = [ "dotmap>=1.3.30",