From 5733e94051e9fc2eb362b53f0fa3a8e39f8aebb7 Mon Sep 17 00:00:00 2001 From: Gui-FernandesBR Date: Mon, 2 Nov 2026 13:21:43 -0300 Subject: [PATCH 1/4] Revert "ENH: list NOAA atmosphere datasets and fetch latest (#660) (#1136)" This reverts commit 24861fdfa40f72b171efbf82f83eb60035b5c3f9. --- CHANGELOG.md | 1 - rocketpy/environment/fetchers/__init__.py | 24 - rocketpy/environment/fetchers/noaa_catalog.py | 411 ------------------ tests/unit/environment/test_fetchers.py | 151 ------- 4 files changed, 587 deletions(-) delete mode 100644 rocketpy/environment/fetchers/noaa_catalog.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 13c598421..9227b66b5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,7 +36,6 @@ Attention: The newest changes should be on top --> - ENH: 3-DOF single rail button flight phase (tip-off analysis), enabled with the opt-in `Flight(use_udot_rail2=True)`. Between the upper rail button leaving the rail and the lower one following it, the rocket pivots about the lower button under a solved constraint wrench instead of jumping straight to free 6-DOF flight. The window is reported as `between_rails_time`, `between_rails_velocity` and `tip_off_duration`, printed by `Flight.info()` and shaded in the attitude plots. Requires `simulation_mode="6 DOF"` and `equations_of_motion="standard"`. [#920](https://github.com/RocketPy-Team/RocketPy/pull/920) - ENH: Support fixed-time parachute deployment triggers [#1133](https://github.com/RocketPy-Team/RocketPy/pull/1133) [#437](https://github.com/RocketPy-Team/RocketPy/issues/437) - DOC: Add SIL parachute ejection integration example [#1131](https://github.com/RocketPy-Team/RocketPy/pull/1131) [#524](https://github.com/RocketPy-Team/RocketPy/issues/524) -- ENH: List NOAA atmosphere datasets and fetch latest [#1136](https://github.com/RocketPy-Team/RocketPy/pull/1136) [#660](https://github.com/RocketPy-Team/RocketPy/issues/660) - ENH: Model unbonded solid-motor grain CM shift [#1138](https://github.com/RocketPy-Team/RocketPy/pull/1138) [#340](https://github.com/RocketPy-Team/RocketPy/issues/340) - ENH: Add `Flight` post-step callback across all phases [#1128](https://github.com/RocketPy-Team/RocketPy/pull/1128) [#758](https://github.com/RocketPy-Team/RocketPy/issues/758) - DOC: Document angle-of-attack drag inputs [#1142](https://github.com/RocketPy-Team/RocketPy/pull/1142) diff --git a/rocketpy/environment/fetchers/__init__.py b/rocketpy/environment/fetchers/__init__.py index 067208717..d74ea55a9 100644 --- a/rocketpy/environment/fetchers/__init__.py +++ b/rocketpy/environment/fetchers/__init__.py @@ -32,19 +32,6 @@ fetch_open_meteo_ensemble, fetch_open_meteo_forecast, ) -from rocketpy.environment.fetchers.noaa_catalog import ( - FORECAST_MODELS_CATALOG_URL, - NOAA_MODEL_COLLECTIONS, - THREDDS_OPENDAP_ROOT, - build_noaa_opendap_url, - collection_catalog_url, - fetch_latest_noaa_dataset, - get_latest_noaa_dataset_identifier, - get_latest_noaa_opendap_url, - list_noaa_atmosphere_datasets, - list_noaa_dataset_identifiers, - resolve_noaa_collection_path, -) from rocketpy.environment.fetchers.opendap_fetchers import ( fetch_aigfs_file_return_dataset, fetch_cmc_ensemble, @@ -64,11 +51,9 @@ __all__ = [ "MAX_RETRY_DELAY_SECONDS", - "FORECAST_MODELS_CATALOG_URL", "METEOMATICS_BASE_URL", "METEOMATICS_LOGIN_URL", "METEOMATICS_TIMEOUT_SECONDS", - "NOAA_MODEL_COLLECTIONS", "OPEN_METEO_ENSEMBLE_MODELS", "OPEN_METEO_ENSEMBLE_URL", "OPEN_METEO_FORECAST_URL", @@ -76,11 +61,8 @@ "OPEN_METEO_HISTORICAL_URL", "OPEN_METEO_PRESSURE_LEVELS", "OPEN_METEO_TIMEOUT_SECONDS", - "THREDDS_OPENDAP_ROOT", "MeteomaticsFetcher", "build_hourly_variables", - "build_noaa_opendap_url", - "collection_catalog_url", "fetch_aigfs_file_return_dataset", "fetch_atmospheric_data_from_meteomatics", "fetch_atmospheric_data_from_windy", @@ -89,7 +71,6 @@ "fetch_gfs_file_return_dataset", "fetch_hiresw_file_return_dataset", "fetch_hrrr_file_return_dataset", - "fetch_latest_noaa_dataset", "fetch_meteomatics_token", "fetch_nam_file_return_dataset", "fetch_open_elevation", @@ -97,13 +78,8 @@ "fetch_open_meteo_forecast", "fetch_rap_file_return_dataset", "fetch_wyoming_sounding", - "get_latest_noaa_dataset_identifier", - "get_latest_noaa_opendap_url", - "list_noaa_atmosphere_datasets", - "list_noaa_dataset_identifiers", "logger", "netCDF4", "requests", - "resolve_noaa_collection_path", "time", ] diff --git a/rocketpy/environment/fetchers/noaa_catalog.py b/rocketpy/environment/fetchers/noaa_catalog.py deleted file mode 100644 index 60b4381b8..000000000 --- a/rocketpy/environment/fetchers/noaa_catalog.py +++ /dev/null @@ -1,411 +0,0 @@ -"""Best-effort helpers for listing NOAA/NCEP atmosphere datasets on THREDDS. - -RocketPy's forecast shortcuts historically hard-coded OPeNDAP URLs (and, for -NOMADS GrADS models, probed calendar times). NOMADS OPeNDAP has been retired, -so discovery is based on the Unidata THREDDS catalogs that already back the -GFS/NAM/RAP/HRRR/AIGFS fetchers. - -Limitations ------------ -- Catalog layout can change without notice; treat results as advisory. -- Listing uses HTTP XML catalogs, not NOMADS OpenDAP directory pages. -- ``fetch_latest_noaa_dataset`` opens the resolved OPeNDAP URL via netCDF4 - (same pattern as the existing ``fetch_*_file_return_dataset`` helpers). It - does not write a local GRIB/NetCDF file to disk. -""" - -from __future__ import annotations - -import re -import time -import xml.etree.ElementTree as ET -from datetime import datetime -from urllib.parse import urljoin, urlparse - -import netCDF4 -import requests - -from rocketpy.environment.fetchers.base import MAX_RETRY_DELAY_SECONDS - -THREDDS_ROOT = "https://thredds.ucar.edu/thredds" -THREDDS_CATALOG_ROOT = f"{THREDDS_ROOT}/catalog/" -THREDDS_OPENDAP_ROOT = f"{THREDDS_ROOT}/dodsC/" -FORECAST_MODELS_CATALOG_URL = f"{THREDDS_CATALOG_ROOT}idd/forecastModels.xml" - -# Collections used by Environment's latest-model shortcuts. -NOAA_MODEL_COLLECTIONS = { - "GFS": "grib/NCEP/GFS/Global_0p25deg", - "NAM": "grib/NCEP/NAM/CONUS_12km", - "RAP": "grib/NCEP/RAP/CONUS_13km", - "HRRR": "grib/NCEP/HRRR/CONUS_2p5km", - "AIGFS": "grib/NCEP/AIGFS/Global_0p25deg", -} - -_THREDDS_NS = { - "thredds": "http://www.unidata.ucar.edu/namespaces/thredds/InvCatalog/v1.0", - "xlink": "http://www.w3.org/1999/xlink", -} -_RUN_TIMESTAMP_RE = re.compile(r"(?P\d{8}_\d{4})") -_DEFAULT_TIMEOUT_SECONDS = 30 - - -def _local_tag(tag: str) -> str: - """Strip Clark-notation namespace from an ElementTree tag.""" - if "}" in tag: - return tag.rsplit("}", 1)[-1] - return tag - - -def _absolute_catalog_url(href: str, base_url: str) -> str: - """Resolve a catalogRef href against a catalog URL.""" - if href.startswith("http://") or href.startswith("https://"): - return href - if href.startswith("/"): - parsed = urlparse(base_url) - return f"{parsed.scheme}://{parsed.netloc}{href}" - return urljoin(base_url, href) - - -def _collection_path_from_catalog_url(catalog_url: str) -> str | None: - """Extract ``grib/NCEP/...`` path from a THREDDS catalog URL, if present.""" - marker = "/catalog/" - if marker not in catalog_url: - return None - path = catalog_url.split(marker, 1)[1] - if path.endswith("/catalog.xml"): - path = path[: -len("/catalog.xml")] - elif path.endswith("catalog.xml"): - path = path[: -len("catalog.xml")].rstrip("/") - return path or None - - -def _fetch_catalog_xml(catalog_url: str, timeout: float = _DEFAULT_TIMEOUT_SECONDS): - """GET a THREDDS catalog XML document and return its root element.""" - try: - response = requests.get(catalog_url, timeout=timeout) - except requests.exceptions.RequestException as exc: - raise RuntimeError( - f"Unable to reach NOAA/THREDDS catalog at {catalog_url}." - ) from exc - - if response.status_code != 200: - raise RuntimeError( - "Unable to list NOAA/THREDDS datasets: " - f"HTTP {response.status_code} for {catalog_url}." - ) - - try: - return ET.fromstring(response.content) - except ET.ParseError as exc: - raise RuntimeError( - f"Invalid THREDDS catalog XML received from {catalog_url}." - ) from exc - - -def _opendap_url_from_path(url_path: str) -> str: - """Build an OPeNDAP URL for a THREDDS dataset ``urlPath``.""" - return urljoin(THREDDS_OPENDAP_ROOT, url_path.lstrip("/")) - - -def _run_sort_key(identifier: str): - """Sort key that prefers identifiers embedding ``YYYYMMDD_HHMM``.""" - match = _RUN_TIMESTAMP_RE.search(identifier) - if match: - try: - return datetime.strptime(match.group("stamp"), "%Y%m%d_%H%M") - except ValueError: - pass - return datetime.min - - -def resolve_noaa_collection_path(model_or_path: str) -> str: - """Map a model shortcut or collection path to a THREDDS collection path. - - Parameters - ---------- - model_or_path : str - Shortcut such as ``\"GFS\"`` or a collection path such as - ``\"grib/NCEP/GFS/Global_0p25deg\"``. - - Returns - ------- - str - Collection path without a leading slash. - - Raises - ------ - ValueError - If ``model_or_path`` is not a known shortcut and does not look like a - collection path. - """ - if not isinstance(model_or_path, str) or not model_or_path.strip(): - raise ValueError("model_or_path must be a non-empty string.") - - key = model_or_path.strip() - mapped = NOAA_MODEL_COLLECTIONS.get(key.upper()) - if mapped is not None: - return mapped - - normalized = key.lstrip("/") - if normalized.startswith("grib/"): - return normalized.removesuffix("/catalog.xml").rstrip("/") - - raise ValueError( - f"Unknown NOAA model collection {model_or_path!r}. " - f"Known shortcuts: {sorted(NOAA_MODEL_COLLECTIONS)}. " - "Pass a THREDDS collection path such as " - "'grib/NCEP/GFS/Global_0p25deg' instead." - ) - - -def collection_catalog_url(model_or_path: str) -> str: - """Return the THREDDS ``catalog.xml`` URL for a model collection.""" - collection = resolve_noaa_collection_path(model_or_path) - return f"{THREDDS_CATALOG_ROOT}{collection}/catalog.xml" - - -def list_noaa_atmosphere_datasets( - catalog_url: str = FORECAST_MODELS_CATALOG_URL, - timeout: float = _DEFAULT_TIMEOUT_SECONDS, -) -> list[dict]: - """List available NCEP atmosphere model collections from THREDDS. - - Parameters - ---------- - catalog_url : str, optional - Root forecast-models catalog URL. Defaults to Unidata's NCEP models - catalog (the same server family used by RocketPy's GFS/NAM/… fetchers). - timeout : float, optional - HTTP timeout in seconds. - - Returns - ------- - list of dict - Each entry has ``name``, ``catalog_url``, ``collection_path`` and - ``opendap_best_url`` (the latter is ``None`` until a collection - catalog is inspected). - """ - root = _fetch_catalog_xml(catalog_url, timeout=timeout) - datasets = [] - seen = set() - - for ref in root.findall(".//thredds:catalogRef", _THREDDS_NS): - href = ref.get(f"{{{_THREDDS_NS['xlink']}}}href") or ref.get("href") - if not href: - continue - name = ( - ref.get(f"{{{_THREDDS_NS['xlink']}}}title") - or ref.get("name") - or ref.get("ID") - or href - ) - absolute = _absolute_catalog_url(href, catalog_url) - collection_path = _collection_path_from_catalog_url(absolute) - key = (name, absolute) - if key in seen: - continue - seen.add(key) - datasets.append( - { - "name": name, - "catalog_url": absolute, - "collection_path": collection_path, - "opendap_best_url": ( - _opendap_url_from_path(f"{collection_path}/Best") - if collection_path - else None - ), - } - ) - - if not datasets: - raise RuntimeError( - f"No NOAA/THREDDS dataset collections found in {catalog_url}." - ) - - return datasets - - -def list_noaa_dataset_identifiers( - model_or_path: str, - timeout: float = _DEFAULT_TIMEOUT_SECONDS, - include_aggregates: bool = True, -) -> list[str]: - """List dataset identifiers available inside a NOAA/NCEP collection. - - Parameters - ---------- - model_or_path : str - Shortcut (``\"GFS\"``, ``\"NAM\"``, …) or a THREDDS collection path. - timeout : float, optional - HTTP timeout in seconds. - include_aggregates : bool, optional - When ``True`` (default), include virtual aggregates such as ``Best`` - and ``TwoD`` alongside individual forecast-run identifiers. - - Returns - ------- - list of str - Identifiers suitable for :func:`build_noaa_opendap_url`. Forecast-run - filenames (``*.grib2``) are sorted newest-first when timestamps can be - parsed; aggregate names are appended afterward. - """ - catalog_url = collection_catalog_url(model_or_path) - root = _fetch_catalog_xml(catalog_url, timeout=timeout) - - runs = [] - aggregates = [] - - for element in root.iter(): - tag = _local_tag(element.tag) - if tag not in {"dataset", "catalogRef"}: - continue - - url_path = element.get("urlPath") - name = ( - element.get(f"{{{_THREDDS_NS['xlink']}}}title") - or element.get("name") - or element.get("ID") - or "" - ) - - if url_path in {None, "", "latest.xml"}: - continue - - identifier = url_path.rsplit("/", 1)[-1] - if identifier.lower().endswith(".grib2") or _RUN_TIMESTAMP_RE.search( - identifier - ): - runs.append(identifier) - elif include_aggregates and identifier in {"Best", "TwoD"}: - aggregates.append(identifier) - elif include_aggregates and name and "Best" in name and identifier: - aggregates.append(identifier) - - # Preserve order while dropping duplicates. - unique_runs = list(dict.fromkeys(runs)) - unique_runs.sort(key=_run_sort_key, reverse=True) - unique_aggregates = list(dict.fromkeys(aggregates)) - identifiers = unique_runs + unique_aggregates - - if not identifiers: - raise RuntimeError( - "No dataset identifiers found in NOAA/THREDDS collection catalog " - f"{catalog_url}." - ) - - return identifiers - - -def build_noaa_opendap_url(model_or_path: str, identifier: str) -> str: - """Build an OPeNDAP URL for a collection dataset identifier.""" - collection = resolve_noaa_collection_path(model_or_path) - return _opendap_url_from_path(f"{collection}/{identifier}") - - -def get_latest_noaa_dataset_identifier( - model_or_path: str, - timeout: float = _DEFAULT_TIMEOUT_SECONDS, -) -> str: - """Return the newest forecast-run identifier for a collection. - - Prefers THREDDS ``latest.xml`` when present. Falls back to the maximum - timestamp among listed ``*.grib2`` runs, then to the ``Best`` aggregate. - - Parameters - ---------- - model_or_path : str - Shortcut or collection path. - timeout : float, optional - HTTP timeout in seconds. - - Returns - ------- - str - Dataset identifier (for example - ``\"GFS_Global_0p25deg_20260810_1800.grib2\"`` or ``\"Best\"``). - """ - collection = resolve_noaa_collection_path(model_or_path) - latest_catalog_url = f"{THREDDS_CATALOG_ROOT}{collection}/latest.xml" - - try: - root = _fetch_catalog_xml(latest_catalog_url, timeout=timeout) - except RuntimeError: - root = None - - if root is not None: - for element in root.iter(): - if _local_tag(element.tag) != "dataset": - continue - url_path = element.get("urlPath") - name = element.get("name") - if url_path and url_path != "latest.xml": - return url_path.rsplit("/", 1)[-1] - if name and name.lower().endswith(".grib2"): - return name - - identifiers = list_noaa_dataset_identifiers( - model_or_path, timeout=timeout, include_aggregates=True - ) - for identifier in identifiers: - if identifier.lower().endswith(".grib2") or _RUN_TIMESTAMP_RE.search( - identifier - ): - return identifier - - if "Best" in identifiers: - return "Best" - - return identifiers[0] - - -def get_latest_noaa_opendap_url( - model_or_path: str = "GFS", - timeout: float = _DEFAULT_TIMEOUT_SECONDS, -) -> str: - """Resolve the OPeNDAP URL for the latest dataset in a collection.""" - identifier = get_latest_noaa_dataset_identifier(model_or_path, timeout=timeout) - return build_noaa_opendap_url(model_or_path, identifier) - - -def fetch_latest_noaa_dataset( - model_or_path: str = "GFS", - max_attempts: int = 10, - base_delay: float = 2, - timeout: float = _DEFAULT_TIMEOUT_SECONDS, -): - """Open the latest NOAA/NCEP dataset for a model via OPeNDAP. - - Parameters - ---------- - model_or_path : str, optional - Shortcut such as ``\"GFS\"`` (default) or a THREDDS collection path. - max_attempts : int, optional - Maximum netCDF4 open attempts. Default is 10. - base_delay : float, optional - Base exponential backoff delay in seconds. Default is 2. - timeout : float, optional - HTTP timeout used while resolving the catalog entry. - - Returns - ------- - netCDF4.Dataset - Open dataset handle. - - Raises - ------ - RuntimeError - If the catalog cannot be resolved or all open attempts fail. - """ - file_url = get_latest_noaa_opendap_url(model_or_path, timeout=timeout) - attempt_count = 0 - while attempt_count < max_attempts: - try: - return netCDF4.Dataset(file_url) - except OSError: - attempt_count += 1 - time.sleep(min(base_delay**attempt_count, MAX_RETRY_DELAY_SECONDS)) - - raise RuntimeError( - "Unable to load the latest NOAA/THREDDS weather dataset through " + file_url - ) diff --git a/tests/unit/environment/test_fetchers.py b/tests/unit/environment/test_fetchers.py index 225f53e5d..f2152f0d7 100644 --- a/tests/unit/environment/test_fetchers.py +++ b/tests/unit/environment/test_fetchers.py @@ -352,157 +352,6 @@ def test_fetch_meteomatics_data_converts_date_to_utc(monkeypatch): assert all("2024-01-01T09:00:00Z" in url for url, _ in data_calls) -_FORECAST_MODELS_XML = """ - - - - -""" - -_GFS_COLLECTION_XML = """ - - - - - - - -""" - -_GFS_LATEST_XML = """ - - - -""" - - -class _FakeCatalogResponse: - """Minimal response object for mocked NOAA/THREDDS catalog GETs.""" - - def __init__(self, text, status_code=200): - self.text = text - self.content = text.encode("utf-8") - self.status_code = status_code - - -def _install_noaa_catalog_mocks(monkeypatch, *, include_latest=True): - """Patch ``requests.get`` with canned forecast-model and GFS catalogs.""" - - def fake_get(url, timeout=None, **_kwargs): - del timeout - if url.endswith("forecastModels.xml") or "idd/forecastModels.xml" in url: - return _FakeCatalogResponse(_FORECAST_MODELS_XML) - if url.endswith("/latest.xml"): - if include_latest: - return _FakeCatalogResponse(_GFS_LATEST_XML) - return _FakeCatalogResponse("missing", status_code=404) - if url.endswith("/Global_0p25deg/catalog.xml"): - return _FakeCatalogResponse(_GFS_COLLECTION_XML) - return _FakeCatalogResponse("unexpected", status_code=404) - - monkeypatch.setattr(fetchers.requests, "get", fake_get) - return fake_get - - -def test_list_noaa_atmosphere_datasets_parses_catalog_refs(monkeypatch): - """List NCEP collections from the mocked forecast-models catalog.""" - _install_noaa_catalog_mocks(monkeypatch) - - datasets = fetchers.list_noaa_atmosphere_datasets() - - assert [entry["name"] for entry in datasets] == [ - "GFS Quarter Degree Forecast", - "NAM CONUS 12km from NOAAPORT", - ] - assert datasets[0]["collection_path"] == "grib/NCEP/GFS/Global_0p25deg" - assert datasets[0]["opendap_best_url"].endswith("grib/NCEP/GFS/Global_0p25deg/Best") - - -def test_list_noaa_dataset_identifiers_sorts_runs_newest_first(monkeypatch): - """GFS run identifiers should sort by embedded timestamp, newest first.""" - _install_noaa_catalog_mocks(monkeypatch) - - identifiers = fetchers.list_noaa_dataset_identifiers("GFS") - - assert identifiers[0] == "GFS_Global_0p25deg_20260810_1800.grib2" - assert identifiers[1] == "GFS_Global_0p25deg_20260810_1200.grib2" - assert "Best" in identifiers - - -def test_get_latest_noaa_opendap_url_uses_latest_xml(monkeypatch): - """Prefer THREDDS latest.xml when resolving the newest GFS run.""" - _install_noaa_catalog_mocks(monkeypatch, include_latest=True) - - url = fetchers.get_latest_noaa_opendap_url("GFS") - - assert url == ( - "https://thredds.ucar.edu/thredds/dodsC/" - "grib/NCEP/GFS/Global_0p25deg/GFS_Global_0p25deg_20260810_1800.grib2" - ) - - -def test_get_latest_noaa_dataset_identifier_falls_back_without_latest_xml( - monkeypatch, -): - """When latest.xml is missing, choose the newest listed grib2 run.""" - _install_noaa_catalog_mocks(monkeypatch, include_latest=False) - - identifier = fetchers.get_latest_noaa_dataset_identifier("gfs") - - assert identifier == "GFS_Global_0p25deg_20260810_1800.grib2" - - -def test_fetch_latest_noaa_dataset_opens_resolved_url(monkeypatch): - """fetch_latest_noaa_dataset should open the resolved OPeNDAP URL.""" - _install_noaa_catalog_mocks(monkeypatch) - calls = [] - sentinel = object() - - def fake_dataset(url): - calls.append(url) - return sentinel - - monkeypatch.setattr(fetchers.netCDF4, "Dataset", fake_dataset) - - dataset = fetchers.fetch_latest_noaa_dataset("GFS", max_attempts=2, base_delay=2) - - assert dataset is sentinel - assert calls == [ - "https://thredds.ucar.edu/thredds/dodsC/" - "grib/NCEP/GFS/Global_0p25deg/GFS_Global_0p25deg_20260810_1800.grib2" - ] - - -def test_resolve_noaa_collection_path_rejects_unknown_model(): - """Unknown shortcuts must fail with an actionable ValueError.""" - with pytest.raises(ValueError, match="Unknown NOAA model collection"): - fetchers.resolve_noaa_collection_path("not-a-model") - - def test_fetch_open_elevation_error_payload_raises_runtime_error(monkeypatch): """An error payload (no ``results``) must surface as the documented RuntimeError.""" payload = {"code": "temporarily_unavailable", "error": "Retry shortly."} From b57465a4d66bc1979eadb927dc085b5c4292cd7d Mon Sep 17 00:00:00 2001 From: Gui-FernandesBR Date: Mon, 2 Nov 2026 13:22:02 -0300 Subject: [PATCH 2/4] Revert "MNT: store ref_factor on rocket aero surface components (#561) (#1129)" This reverts commit bc3fe7357d4bff14923477071417c208d577475d. --- CHANGELOG.md | 1 - rocketpy/plots/flight_plots.py | 2 +- rocketpy/plots/rocket_plots.py | 4 +- rocketpy/prints/rocket_prints.py | 5 +- rocketpy/rocket/components.py | 28 ++---- rocketpy/rocket/rocket.py | 24 ++--- rocketpy/simulation/flight.py | 10 +- rocketpy/stochastic/stochastic_rocket.py | 2 +- tests/unit/rocket/test_components.py | 91 ------------------- tests/unit/rocket/test_rocket.py | 4 +- .../unit/stochastic/test_stochastic_model.py | 2 +- .../test_stochastic_rocket_seeding.py | 4 +- 12 files changed, 31 insertions(+), 146 deletions(-) delete mode 100644 tests/unit/rocket/test_components.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 9227b66b5..7a5c1e3d8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,7 +52,6 @@ Attention: The newest changes should be on top --> ### Changed - ENH: Number the simulations of a serial Monte Carlo run from zero, as the parallel path already did — the two used to name the same simulation `1, 2, 3` and `0, 1, 2`. The `index` field written into the inputs and outputs rows shifts by one for serial runs, so code reading indices off those files has to expect a run of `n` simulations to be numbered `0` to `n - 1`. An append onto a study written before this release is refused rather than continued, since its rows do not record which root drew them. [#1187](https://github.com/RocketPy-Team/RocketPy/pull/1187) -- MNT: Store the reference-area correction factor on each rocket aero surface component, so it is carried with the surface instead of being recomputed at every lift evaluation. `Rocket.aerodynamic_surfaces`, `rail_buttons` and `sensors` now yield `(component, position, ref_factor)`, so code that unpacks a pair from them (`for surface, position in rocket.aerodynamic_surfaces`) has to take the third field or absorb it. Simulation results are unchanged, and `.rpy` files written before this still load. [#1129](https://github.com/RocketPy-Team/RocketPy/pull/1129) [#561](https://github.com/RocketPy-Team/RocketPy/issues/561) - ENH: Compute the rocket static margin lazily [#1135](https://github.com/RocketPy-Team/RocketPy/pull/1135) [#780](https://github.com/RocketPy-Team/RocketPy/issues/780) - DOC: Tighten the comments that came with the sampler seed groups [#1154](https://github.com/RocketPy-Team/RocketPy/pull/1154) - CI: make the Gemini PR reviewer actually review [#1140](https://github.com/RocketPy-Team/RocketPy/pull/1140) diff --git a/rocketpy/plots/flight_plots.py b/rocketpy/plots/flight_plots.py index bbff706c5..2a0c3f164 100644 --- a/rocketpy/plots/flight_plots.py +++ b/rocketpy/plots/flight_plots.py @@ -939,7 +939,7 @@ def _rocket_axial_display_coordinate(self, value, display_length): """Map a rocket axial coordinate onto the centered display model.""" coordinates = [ float(position.z) - for _surface, position, _ref_factor in self.flight.rocket.aerodynamic_surfaces + for _surface, position in self.flight.rocket.aerodynamic_surfaces ] coordinates.extend( [ diff --git a/rocketpy/plots/rocket_plots.py b/rocketpy/plots/rocket_plots.py index 5f55eaaab..8e2b35558 100644 --- a/rocketpy/plots/rocket_plots.py +++ b/rocketpy/plots/rocket_plots.py @@ -243,7 +243,7 @@ def _draw_aerodynamic_surfaces(self, ax, vis_args, plane, surfaces): # diameter changes. The final point of the last surface is the final # point of the last tube - for surface, position, _ref_factor in surfaces: + for surface, position in surfaces: if isinstance(surface, NoseCone): self._draw_nose_cone(ax, surface, position.z, drawn_surfaces, vis_args) elif isinstance(surface, Tail): @@ -645,7 +645,7 @@ def _draw_nozzle_tube(self, last_radius, last_x, nozzle_position, ax, vis_args): def _draw_rail_buttons(self, ax, vis_args): """Draws the rail buttons of the rocket.""" try: - buttons, pos, _ref_factor = self.rocket.rail_buttons[0] + buttons, pos = self.rocket.rail_buttons[0] lower = pos.z upper = lower + buttons.buttons_distance * self.rocket._csys ax.scatter( diff --git a/rocketpy/prints/rocket_prints.py b/rocketpy/prints/rocket_prints.py index 6c0c667d4..7b768ea2f 100644 --- a/rocketpy/prints/rocket_prints.py +++ b/rocketpy/prints/rocket_prints.py @@ -102,18 +102,19 @@ def rocket_aerodynamics_quantities(self): None """ print("\nAerodynamics Lift Coefficient Derivatives\n") - for surface, _position, ref_factor in self.rocket.aerodynamic_surfaces: + for surface, _ in self.rocket.aerodynamic_surfaces: if isinstance(surface, GenericSurface): continue name = surface.name # ref_factor corrects lift for different reference areas + ref_factor = (surface.rocket_radius / self.rocket.radius) ** 2 print( f"{name} Lift Coefficient Derivative: " f"{ref_factor * surface.clalpha(0):.3f}/rad" ) print("\nCenter of Pressure\n") - for surface, position, _ref_factor in self.rocket.aerodynamic_surfaces: + for surface, position in self.rocket.aerodynamic_surfaces: name = surface.name cpz = surface.cp[2] # relative to the user defined coordinate system print( diff --git a/rocketpy/rocket/components.py b/rocketpy/rocket/components.py index a6d013b9b..57e4d12f8 100644 --- a/rocketpy/rocket/components.py +++ b/rocketpy/rocket/components.py @@ -15,15 +15,13 @@ class Components: A list of named tuples representing all the components and their positions relative to the rocket. component_tuple : namedtuple - A named tuple representing a component, its position within the - rocket, and an optional reference-area correction factor. + A named tuple representing a component and its position within the + rocket. """ def __init__(self): """Initialize an empty components list instance.""" - self.component_tuple = namedtuple( - "component_tuple", "component position ref_factor", defaults=(1.0,) - ) + self.component_tuple = namedtuple("component_tuple", "component position") self._components = [] # List of components and their positions to avoid extra for loops in @@ -36,7 +34,6 @@ def __repr__(self): components_str = "\n".join( [ f"\tComponent: {str(c.component):80} Position: {c.position}" - f" Ref Factor: {c.ref_factor}" for c in self._components ] ) @@ -55,7 +52,7 @@ def __iter__(self): """Return an iterator over the list of components.""" return iter(self._components) - def add(self, component, position, ref_factor=1.0): + def add(self, component, position): """Add a component to the list of components. Parameters @@ -65,9 +62,6 @@ def add(self, component, position, ref_factor=1.0): position : int, float The position of the component relative to the rocket's coordinate system origin. - ref_factor : int, float, optional - Reference-area correction factor associating the component to the - rocket reference area. Defaults to 1.0 when not applicable. Returns ------- @@ -75,7 +69,7 @@ def add(self, component, position, ref_factor=1.0): """ self.__component_list.append(component) self.__position_list.append(position) - self._components.append(self.component_tuple(component, position, ref_factor)) + self._components.append(self.component_tuple(component, position)) def get_by_type(self, component_type): """Search the list of components and return a list with all the @@ -213,11 +207,7 @@ def sort_by_position(self, reverse=False): def to_dict(self, **kwargs): # pylint: disable=unused-argument return { "components": [ - { - "component": c.component, - "position": c.position, - "ref_factor": c.ref_factor, - } + {"component": c.component, "position": c.position} for c in self._components ] } @@ -226,9 +216,5 @@ def to_dict(self, **kwargs): # pylint: disable=unused-argument def from_dict(cls, data): components = cls() for component in data["components"]: - components.add( - component["component"], - component["position"], - ref_factor=component.get("ref_factor", 1.0), - ) + components.add(component["component"], component["position"]) return components diff --git a/rocketpy/rocket/rocket.py b/rocketpy/rocket/rocket.py index f973677fd..602ad1543 100644 --- a/rocketpy/rocket/rocket.py +++ b/rocketpy/rocket/rocket.py @@ -667,10 +667,11 @@ def evaluate_center_of_pressure(self): # Calculate total lift coefficient derivative and center of pressure if len(self.aerodynamic_surfaces) > 0: - for aero_surface, position, ref_factor in self.aerodynamic_surfaces: + for aero_surface, position in self.aerodynamic_surfaces: if isinstance(aero_surface, GenericSurface): continue # ref_factor corrects lift for different reference areas + ref_factor = (aero_surface.rocket_radius / self.radius) ** 2 self.total_lift_coeff_der += ref_factor * aero_surface.clalpha self.cp_position += ( ref_factor @@ -693,7 +694,7 @@ def evaluate_surfaces_cp_to_cdm(self): Dictionary mapping the relative position of each aerodynamic surface center of pressure to the rocket's center of mass. """ - for surface, position, _ref_factor in self.aerodynamic_surfaces: + for surface, position in self.aerodynamic_surfaces: self.__evaluate_single_surface_cp_to_cdm(surface, position) return self.surfaces_cp_to_cdm @@ -818,7 +819,7 @@ def warn_if_unstable(self): """ has_generic_surface = any( isinstance(aero_surface, GenericSurface) - for aero_surface, _position, _ref_factor in self.aerodynamic_surfaces + for aero_surface, _position in self.aerodynamic_surfaces ) if has_generic_surface: return False @@ -1193,12 +1194,7 @@ def __add_single_surface(self, surface, position): self.rail_buttons = Components() self.rail_buttons.add(surface, position) else: - # ref_factor corrects lift for different reference areas - if getattr(surface, "rocket_radius", None) is not None: - ref_factor = (surface.rocket_radius / self.radius) ** 2 - else: - ref_factor = 1.0 - self.aerodynamic_surfaces.add(surface, position, ref_factor=ref_factor) + self.aerodynamic_surfaces.add(surface, position) self.__evaluate_single_surface_cp_to_cdm(surface, position) def add_surfaces(self, surfaces, positions): @@ -2371,14 +2367,10 @@ def from_dict(cls, data): position=data["motor_position"], ) - # The trailing star reads entries of either length: files written - # before ref_factor was stored carry two fields and newer ones three. - # The factor is not read back in any case, since add_surfaces derives - # it from the surface's own radius. - for surface, position, *_ in data["aerodynamic_surfaces"]: + for surface, position in data["aerodynamic_surfaces"]: rocket.add_surfaces(surfaces=surface, positions=position) - for button, position, *_ in data["rail_buttons"]: + for button, position in data["rail_buttons"]: rocket.set_rail_buttons( upper_button_position=position[2] + button.buttons_distance, lower_button_position=position[2], @@ -2389,7 +2381,7 @@ def from_dict(cls, data): for parachute in data["parachutes"]: rocket.parachutes.append(parachute) - for sensor, position, *_ in data["sensors"]: + for sensor, position in data["sensors"]: rocket.add_sensor(sensor, position) for air_brake in data["air_brakes"]: diff --git a/rocketpy/simulation/flight.py b/rocketpy/simulation/flight.py index 43790110c..d0b8a5aea 100644 --- a/rocketpy/simulation/flight.py +++ b/rocketpy/simulation/flight.py @@ -889,7 +889,7 @@ def __measure_sensors(self, component_sensors, u_dot, t=None, y_sol=None): Parameters ---------- component_sensors : list - List of (sensor, position, ref_factor) component tuples. + List of (sensor, position) tuples. u_dot : array_like State derivative vector. t : float, optional @@ -902,7 +902,7 @@ def __measure_sensors(self, component_sensors, u_dot, t=None, y_sol=None): if y_sol is None: y_sol = self.y_sol - for sensor, position, _ref_factor in component_sensors: + for sensor, position in component_sensors: relative_position = position - self.rocket._csys * Vector( [0, 0, self.rocket.center_of_dry_mass_position] ) @@ -2338,7 +2338,7 @@ def u_dot(self, t, u, post_processing=False): # pylint: disable=too-many-locals # Calculate lift and moment for each component of the rocket velocity_in_body_frame = Vector([vx_b, vy_b, vz_b]) w = Vector([omega1, omega2, omega3]) - for aero_surface, _, _ref_factor in self.rocket.aerodynamic_surfaces: + for aero_surface, _ in self.rocket.aerodynamic_surfaces: # Component cp relative to CDM in body frame comp_cp = self.rocket.surfaces_cp_to_cdm[aero_surface] # Component absolute velocity in body frame @@ -2594,7 +2594,7 @@ def u_dot_generalized_3dof(self, t, u, post_processing=False): # Velocity in body frame vb_body = Kt @ v - for surface, _, _ref_factor in self.rocket.aerodynamic_surfaces: + for surface, _ in self.rocket.aerodynamic_surfaces: cp = self.rocket.surfaces_cp_to_cdm[surface] vb_component = vb_body + (w ^ cp) @@ -2866,7 +2866,7 @@ def u_dot_generalized(self, t, u, post_processing=False): # pylint: disable=too # Get rocket velocity in body frame velocity_in_body_frame = Kt @ v # Calculate lift and moment for each component of the rocket - for aero_surface, _, _ref_factor in self.rocket.aerodynamic_surfaces: + for aero_surface, _ in self.rocket.aerodynamic_surfaces: # Component cp relative to CDM in body frame comp_cp = self.rocket.surfaces_cp_to_cdm[aero_surface] # Component absolute velocity in body frame diff --git a/rocketpy/stochastic/stochastic_rocket.py b/rocketpy/stochastic/stochastic_rocket.py index 3aa234216..03154eabe 100644 --- a/rocketpy/stochastic/stochastic_rocket.py +++ b/rocketpy/stochastic/stochastic_rocket.py @@ -233,7 +233,7 @@ def __reset_components(self, components, root): input component. """ new_components = Components() - for stochastic_obj, _position, _ref_factor in components: + for stochastic_obj, _ in components: stochastic_obj_position_info = self.__components_map[stochastic_obj] stochastic_obj._set_stochastic(_seed_sequence_to_int(root.spawn(1)[0])) new_components.add( diff --git a/tests/unit/rocket/test_components.py b/tests/unit/rocket/test_components.py deleted file mode 100644 index d09386a4a..000000000 --- a/tests/unit/rocket/test_components.py +++ /dev/null @@ -1,91 +0,0 @@ -"""Unit tests for rocketpy.rocket.components.Components.""" - -import pytest - -from rocketpy import NoseCone -from rocketpy.mathutils.vector_matrix import Vector -from rocketpy.rocket.components import Components - - -def test_components_add_default_ref_factor(): - """Components.add stores ref_factor=1.0 when omitted.""" - components = Components() - component = object() - position = Vector([0, 0, 1.0]) - - components.add(component, position) - - assert len(components) == 1 - assert components[0].component is component - assert components[0].position == position - assert components[0].ref_factor == pytest.approx(1.0) - - -def test_components_add_custom_ref_factor(): - """Components.add stores an explicit ref_factor on the component tuple.""" - components = Components() - component = object() - position = Vector([0, 0, -0.5]) - ref_factor = 0.25 - - components.add(component, position, ref_factor=ref_factor) - - stored = components[0] - assert stored.component is component - assert stored.position == position - assert stored.ref_factor == pytest.approx(ref_factor) - - -def test_components_to_dict_from_dict_preserves_ref_factor(): - """Serialization round-trip keeps ref_factor, defaulting when absent.""" - components = Components() - component = {"name": "dummy"} - position = Vector([0, 0, 0.1]) - components.add(component, position, ref_factor=4.0) - - restored = Components.from_dict(components.to_dict()) - assert restored[0].ref_factor == pytest.approx(4.0) - - legacy = Components.from_dict( - {"components": [{"component": component, "position": position}]} - ) - assert legacy[0].ref_factor == pytest.approx(1.0) - - -def test_rocket_add_surfaces_stores_computed_ref_factor(calisto): - """Rocket aero-surface add path stores (surface.rocket_radius / rocket.radius)**2.""" - surface_radius = calisto.radius / 2 - expected_ref_factor = (surface_radius / calisto.radius) ** 2 - nose = NoseCone( - length=0.55829, - kind="vonkarman", - base_radius=surface_radius, - rocket_radius=surface_radius, - name="Half-radius nose", - ) - - calisto.add_surfaces(nose, 1.16) - stored = next( - entry for entry in calisto.aerodynamic_surfaces if entry.component is nose - ) - - assert stored.ref_factor == pytest.approx(expected_ref_factor) - assert stored.ref_factor == pytest.approx(0.25) - - -def test_rocket_add_surfaces_matching_radius_stores_unit_ref_factor(calisto): - """Matching surface and rocket radii store ref_factor of 1.0.""" - nose = NoseCone( - length=0.55829, - kind="vonkarman", - base_radius=calisto.radius, - rocket_radius=calisto.radius, - name="Matching nose", - ) - - calisto.add_surfaces(nose, 1.16) - stored = next( - entry for entry in calisto.aerodynamic_surfaces if entry.component is nose - ) - - assert stored.ref_factor == pytest.approx(1.0) diff --git a/tests/unit/rocket/test_rocket.py b/tests/unit/rocket/test_rocket.py index 5ac011a8d..ec3123317 100644 --- a/tests/unit/rocket/test_rocket.py +++ b/tests/unit/rocket/test_rocket.py @@ -994,9 +994,7 @@ def test_add_trapezoidal_fins_two_fins_warns_but_succeeds(calisto): fins = calisto.add_trapezoidal_fins( 2, span=0.1, root_chord=0.12, tip_chord=0.04, position=-1.0 ) - assert fins in [ - surface for surface, _position, _ref_factor in calisto.aerodynamic_surfaces - ] + assert fins in [surface for surface, _ in calisto.aerodynamic_surfaces] def test_unstable_rocket_warning_raised(calisto): diff --git a/tests/unit/stochastic/test_stochastic_model.py b/tests/unit/stochastic/test_stochastic_model.py index a8691af6b..96bf04b3c 100644 --- a/tests/unit/stochastic/test_stochastic_model.py +++ b/tests/unit/stochastic/test_stochastic_model.py @@ -320,7 +320,7 @@ def test_two_components_do_not_share_one_position_nominal(stochastic_calisto): stochastic_calisto._set_stochastic(5) places = {} - for component, position, *_ in stochastic_calisto.aerodynamic_surfaces: + for component, position in stochastic_calisto.aerodynamic_surfaces: nominal = position[0] places[type(component).__name__] = float(getattr(nominal, "z", nominal)) diff --git a/tests/unit/stochastic/test_stochastic_rocket_seeding.py b/tests/unit/stochastic/test_stochastic_rocket_seeding.py index 151bbc256..a3430762b 100644 --- a/tests/unit/stochastic/test_stochastic_rocket_seeding.py +++ b/tests/unit/stochastic/test_stochastic_rocket_seeding.py @@ -39,9 +39,9 @@ def _drawn(component): def _members_of(collection): - """Components yields (component, position, ref_factor); a list does not.""" + """Components yields (component, position) pairs; a plain list does not.""" if isinstance(collection, Components): - return [component for component, *_ in collection] + return [component for component, _ in collection] return list(collection) From 97555d82a93251c6c1f0012beecbbe0c98b00639 Mon Sep 17 00:00:00 2001 From: Gui-FernandesBR Date: Mon, 2 Nov 2026 13:24:30 -0300 Subject: [PATCH 3/4] MNT: compute the reference-area factor on demand in one helper Rocket and its prints computed the same factor inline. The test pins that changing a surface's rocket_radius after adding it moves the static margin. Related to #1209 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01XDW574AQLHTSeBM9K172TX --- rocketpy/prints/rocket_prints.py | 3 +-- rocketpy/rocket/rocket.py | 8 ++++++-- tests/unit/rocket/test_rocket.py | 33 ++++++++++++++++++++++++++++++++ 3 files changed, 40 insertions(+), 4 deletions(-) diff --git a/rocketpy/prints/rocket_prints.py b/rocketpy/prints/rocket_prints.py index 7b768ea2f..32df02b39 100644 --- a/rocketpy/prints/rocket_prints.py +++ b/rocketpy/prints/rocket_prints.py @@ -106,8 +106,7 @@ def rocket_aerodynamics_quantities(self): if isinstance(surface, GenericSurface): continue name = surface.name - # ref_factor corrects lift for different reference areas - ref_factor = (surface.rocket_radius / self.rocket.radius) ** 2 + ref_factor = self.rocket._reference_area_factor(surface) print( f"{name} Lift Coefficient Derivative: " f"{ref_factor * surface.clalpha(0):.3f}/rad" diff --git a/rocketpy/rocket/rocket.py b/rocketpy/rocket/rocket.py index 602ad1543..b83d67d4c 100644 --- a/rocketpy/rocket/rocket.py +++ b/rocketpy/rocket/rocket.py @@ -670,8 +670,7 @@ def evaluate_center_of_pressure(self): for aero_surface, position in self.aerodynamic_surfaces: if isinstance(aero_surface, GenericSurface): continue - # ref_factor corrects lift for different reference areas - ref_factor = (aero_surface.rocket_radius / self.radius) ** 2 + ref_factor = self._reference_area_factor(aero_surface) self.total_lift_coeff_der += ref_factor * aero_surface.clalpha self.cp_position += ( ref_factor @@ -683,6 +682,11 @@ def evaluate_center_of_pressure(self): self.cp_position /= self.total_lift_coeff_der return self.cp_position + def _reference_area_factor(self, surface): + """Scales a surface's lift coefficient from its own reference area to + the rocket's. Computed on demand so it follows later radius changes.""" + return (surface.rocket_radius / self.radius) ** 2 + def evaluate_surfaces_cp_to_cdm(self): """Calculates the relative position of each aerodynamic surface center of pressure to the rocket's center of dry mass in Body Axes Coordinate diff --git a/tests/unit/rocket/test_rocket.py b/tests/unit/rocket/test_rocket.py index ec3123317..fd89c429d 100644 --- a/tests/unit/rocket/test_rocket.py +++ b/tests/unit/rocket/test_rocket.py @@ -1,3 +1,4 @@ +import copy import warnings from itertools import product from unittest.mock import patch @@ -1125,3 +1126,35 @@ def test_evaluate_reduced_mass_with_motor(calisto): for t in [0.0, 1.0, 2.0, 3.0]: expected = prop_mass(t) * dry_mass / (prop_mass(t) + dry_mass) assert reduced_mass(t) == pytest.approx(expected) + + +def test_static_margin_follows_surface_radius_changed_after_adding( + calisto_motorless, +): + """A surface's reference-area factor must reflect its current + rocket_radius, not the one it had when it was added.""" + + def add_aero(rocket, nose_radius): + nose = NoseCone( + length=0.55829, + kind="vonkarman", + base_radius=nose_radius, + rocket_radius=nose_radius, + ) + rocket.add_surfaces(nose, 1.278) + rocket.add_trapezoidal_fins( + 4, root_chord=0.120, tip_chord=0.060, span=0.110, position=-1.04956 + ) + return nose + + fresh = copy.deepcopy(calisto_motorless) + add_aero(fresh, 0.0635) + + changed = calisto_motorless + nose = add_aero(changed, 0.03) + nose.base_radius = 0.0635 + nose.rocket_radius = 0.0635 + changed.evaluate_center_of_pressure() + changed.evaluate_static_margin() + + assert changed.static_margin(0) == pytest.approx(fresh.static_margin(0)) From c000b6bf9d52c1e05a427abebbad4a45d425b823 Mon Sep 17 00:00:00 2001 From: Gui-FernandesBR Date: Mon, 2 Nov 2026 13:24:48 -0300 Subject: [PATCH 4/4] Revert "ENH: add Flight post-step callback across all phases (#758) (#1128)" This reverts commit 826531bc627d3a03648e619aa35be248257eb8f9. --- CHANGELOG.md | 1 - rocketpy/simulation/flight.py | 22 --------------- tests/unit/simulation/test_flight.py | 41 ---------------------------- 3 files changed, 64 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7a5c1e3d8..5631b7494 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,7 +37,6 @@ Attention: The newest changes should be on top --> - ENH: Support fixed-time parachute deployment triggers [#1133](https://github.com/RocketPy-Team/RocketPy/pull/1133) [#437](https://github.com/RocketPy-Team/RocketPy/issues/437) - DOC: Add SIL parachute ejection integration example [#1131](https://github.com/RocketPy-Team/RocketPy/pull/1131) [#524](https://github.com/RocketPy-Team/RocketPy/issues/524) - ENH: Model unbonded solid-motor grain CM shift [#1138](https://github.com/RocketPy-Team/RocketPy/pull/1138) [#340](https://github.com/RocketPy-Team/RocketPy/issues/340) -- ENH: Add `Flight` post-step callback across all phases [#1128](https://github.com/RocketPy-Team/RocketPy/pull/1128) [#758](https://github.com/RocketPy-Team/RocketPy/issues/758) - DOC: Document angle-of-attack drag inputs [#1142](https://github.com/RocketPy-Team/RocketPy/pull/1142) - ENH: Create ensembles from user-defined profiles [#1141](https://github.com/RocketPy-Team/RocketPy/pull/1141) - ENH: Add tube-fin aerodynamic surface [#1144](https://github.com/RocketPy-Team/RocketPy/pull/1144) diff --git a/rocketpy/simulation/flight.py b/rocketpy/simulation/flight.py index d0b8a5aea..322548eb2 100644 --- a/rocketpy/simulation/flight.py +++ b/rocketpy/simulation/flight.py @@ -79,14 +79,6 @@ class Flight: Name of the flight. Flight._controllers : list List of controllers to be used during simulation. - Flight.post_step_callback : callable, optional - Optional callback invoked once after every successful ODE solver - step for the entire simulation, including parachute descent. - Receives the ``Flight`` instance (``callback(flight)``). Use - ``flight.t`` and ``flight.y_sol`` for the current time and state. - Controllers stop being useful after parachute deployment; this - callback is the extension point for full-lifecycle observers - (e.g. ground-station / radio update simulation). Flight.max_time : int, float Maximum simulation time allowed. Refers to physical time being simulated, not time taken to run simulation. @@ -536,7 +528,6 @@ def __init__( # pylint: disable=too-many-arguments,too-many-statements equations_of_motion="standard", ode_solver="LSODA", simulation_mode="6 DOF", - post_step_callback=None, use_udot_rail2=False, ): """Run a trajectory simulation. @@ -624,12 +615,6 @@ def __init__( # pylint: disable=too-many-arguments,too-many-statements simulation_mode : str, optional Simulation mode to use. Can be "6 DOF" for 6 degrees of freedom or "3 DOF" for 3 degrees of freedom. Default is "6 DOF". - post_step_callback : callable, optional - Callback invoked once after every successful ODE solver step for - the entire simulation, including parachute phases. Signature is - ``callback(flight)``, matching phase/node callbacks. Access the - current time and state via ``flight.t`` and ``flight.y_sol``. - Default is None. use_udot_rail2 : bool, optional If True, enable the intermediate 3-DOF "tip-off" rail phase ``udot_rail2``: after the upper rail button leaves the rail (at @@ -647,9 +632,6 @@ def __init__( # pylint: disable=too-many-arguments,too-many-statements ---------- .. [1] https://docs.scipy.org/doc/scipy/reference/generated/scipy.integrate.solve_ivp.html """ - if post_step_callback is not None and not callable(post_step_callback): - raise TypeError("post_step_callback must be callable or None") - # Save arguments self.env = environment self.rocket = rocket @@ -676,7 +658,6 @@ def __init__( # pylint: disable=too-many-arguments,too-many-statements self.equations_of_motion = equations_of_motion self.simulation_mode = simulation_mode self.ode_solver = ode_solver - self.post_step_callback = post_step_callback # Enable or disable the intermediate 3-DOF rail phase self.use_udot_rail2 = use_udot_rail2 @@ -788,9 +769,6 @@ def __simulate(self, verbose): self.sensors, self.env, ) - # Full-lifecycle observer (all phases, including parachute) - if self.post_step_callback is not None: - self.post_step_callback(self) if self.__check_simulation_events(phase, phase_index, node_index): break # Stop if simulation termination event occurred diff --git a/tests/unit/simulation/test_flight.py b/tests/unit/simulation/test_flight.py index 64082f7a4..10928e02f 100644 --- a/tests/unit/simulation/test_flight.py +++ b/tests/unit/simulation/test_flight.py @@ -235,47 +235,6 @@ def test_initial_solution_from_flight_sets_initial_time( assert second.t_initial == first.solution[-1][0] -def test_post_step_callback_runs_before_and_after_apogee( - calisto_robust, example_plain_env -): - """post_step_callback must fire across the full flight, including descent. - - Controllers are not a substitute: air-brake fixtures often terminate at - apogee, and parachute phases are not meant to keep feeding actuators. - This callback is the full-lifecycle observer hook (issue #758). - """ - callback_times = [] - - def record_step(flight): - callback_times.append(flight.t) - - flight = Flight( - rocket=calisto_robust, - environment=example_plain_env, - rail_length=5.2, - inclination=85, - heading=0, - terminate_on_apogee=False, - post_step_callback=record_step, - ) - - assert callback_times, "post_step_callback was never invoked" - assert min(callback_times) < flight.apogee_time - assert max(callback_times) > flight.apogee_time - assert flight.t_final > flight.apogee_time - - -def test_post_step_callback_must_be_callable(calisto, example_plain_env): - """Non-callable post_step_callback values are rejected at construction.""" - with pytest.raises(TypeError, match="post_step_callback"): - Flight( - rocket=calisto, - environment=example_plain_env, - rail_length=5.2, - post_step_callback="not-callable", - ) - - def test_initial_stability_margin(flight_calisto_custom_wind): """Test the initial_stability_margin method of the Flight class.