Skip to content

Data/tropical cyclone hazard logic - #306

Open
jannisvisser wants to merge 55 commits into
mainfrom
data/tropical-cyclone-hazard-logic
Open

Data/tropical cyclone hazard logic#306
jannisvisser wants to merge 55 commits into
mainfrom
data/tropical-cyclone-hazard-logic

Conversation

@jannisvisser

@jannisvisser jannisvisser commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

AB#42859

Describe your changes

Status 31 July written by Jannis:

  • Full version done by @duruena
  • Including 3 test scenarios (see TESTING_SCENARIOS.md), which require test data from here. Specifically, both from gefs_track and gefs_wind the subfolders gefs.20210512, gefs.20240929 and gefs.20250918.
  • Approved by Jacopo from code-review
  • 2 rounds of review+discussion by Jannis from code+functional review ready. Current a handful of conversations open. None big. Given the logistical context, dev can also play a role in resolving these (in this or new PR).
  • Note that in general, there is a substantial list of todo-infra tasks, collected in here. These can be implemented after this PR.
  • Dev role is handed over to Gulfaraz.

Checklist before requesting a code review

  • I have performed a self-review of my code
  • I have addressed all Copilot comments
  • I have asked the design team to review these changes, or: The changes do not touch the UI/UX
  • I have added tests for my changes, or: Adding tests is unnecessary/irrelevant
  • I have made sure that all automated checks pass before requesting a review
  • I have updated all documentation where necessary
  • I do not need any deviation from our PR guidelines

Portal preview-deployment

This PR does not have any preview deployments yet.

jmargutt and others added 16 commits June 29, 2026 14:56
Add the tropical-cyclone hazard type, forecast source, and wind-speed severity/layer keys to the shared enums, and rename the pipeline package folder to an importable name. Add the Philippines pipeline config (Typhoon Goni study case), sourcing only admin-area and population data for now

Python enums.py regeneration (npm run generate:python) is pending
…dd basin-pluggable Saffir-Simpson/PAGASA wind-category tables and rewrite forecast.py into the full step-by-step orchestration, with placeholder functions standing in for extract_forecast.py/determine_alerts.py/ compute_wind_extent.py/determine_exposure.py (none exist yet.
… now a single flat constant. Integrate ATCF track data into the orchestration. Replace manually-configured per-country bounding boxes with a computed value: admin-area bounding box padded by a shared MONITORING_BOX_BUFFER_KM.
…ropicalCyclone, gefs, and windSpeed to schema.prisma. Register the hazard in run_forecasts.py. Fix a real bug in config_reader.py found along the way: it blanket-lowercased the YAML hazard_type before matching, which can never match a value containing a capital letter.
Copilot AI review requested due to automatic review settings July 13, 2026 14:55
@jannisvisser
jannisvisser marked this pull request as draft July 13, 2026 14:55
@jannisvisser

Copy link
Copy Markdown
Contributor Author

@duruena FYI I have already created a (draft) PR out of this branch for clarity.

@jannisvisser jannisvisser added the enhancement New feature or request that affects our end users label Jul 13, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces initial tropical cyclone hazard support across the IBF stack: registering new hazard-related enums in the API database layer and adding a new tropical-cyclone pipeline implementation under data/pipelines/.

Changes:

  • Register tropical cyclone-related enum values in Prisma/PostgreSQL (HazardType, ForecastSource, SeverityKey, LayerName).
  • Add a new data/pipelines/tropical_cyclone/ hazard implementation (forecast orchestration + wind/track extraction + alert/extent/exposure logic).
  • Wire the new hazard into the pipeline framework (config, dispatch registration, enums, logging tag) and document camelCase hazard_type handling.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
services/api-service/src/prisma/schema.prisma Adds new enum values needed for tropical cyclone hazard + GEFS + wind-speed layers.
services/api-service/src/prisma/migrations/20260708130541_add_tropical_cyclone_hazard_enums/migration.sql DB migration to add the new enum values in Postgres.
data/pipelines/tropical_cyclone/README.md Introduces hazard-specific README for the new pipeline folder.
data/pipelines/tropical_cyclone/init.py Exposes the tropical cyclone package module(s).
data/pipelines/tropical_cyclone/forecast.py Main tropical cyclone orchestration: load inputs, compute severities/extents/exposure, submit outputs.
data/pipelines/tropical_cyclone/constants.py Adds tropical cyclone domain constants and country-specific configuration.
data/pipelines/tropical_cyclone/extract_forecast.py Implements GEFS wind extraction and conversion into sustained wind speed rasters.
data/pipelines/tropical_cyclone/extract_track.py Implements GEFS ATCF track parsing into per-lead-time fixes within bounds.
data/pipelines/tropical_cyclone/determine_alerts.py Determines alert time buckets from land-masked ensemble maxima and medians.
data/pipelines/tropical_cyclone/compute_wind_extent.py Computes a precautionary wind extent raster envelope for the peak bucket.
data/pipelines/tropical_cyclone/determine_exposure.py Clips the computed extent to the configured/admin-area footprint.
data/pipelines/README.md Documents hazard_type exact matching and camelCase for multi-word hazards.
data/pipelines/infra/utils/nrw_logger.py Adds a new LogTag for tropical cyclone hazard logging.
data/pipelines/infra/data_types/enums.py Adds HazardType.TROPICAL_CYCLONE, ForecastSource.GEFS, and wind-speed layer/severity enums.
data/pipelines/infra/run_forecasts.py Registers and dispatches the new tropicalCyclone hazard function and forecast source mapping.
data/pipelines/infra/configs/tropicalCyclone.yaml Adds a first hazard config stub for tropicalCyclone (currently infra-only).
data/pipelines/infra/config_reader.py Stops lowercasing hazard_type, enforcing exact enum value matching.

Comment thread data/pipelines/tropical_cyclone/README.md Outdated
Comment thread data/pipelines/tropical_cyclone/extract_forecast.py
Comment thread data/pipelines/tropical_cyclone/extract_forecast.py
duruena and others added 8 commits July 14, 2026 10:03
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…ind speed by config

PR #307 seeded a real per-country AlertConfig (spatial + temporal extent)
for tropicalCyclone, so stop synthesizing one locally: forecast.py now
fetches DataSource.ALERT_CONFIGS_IBF_API (guarded like flood/drought) and
loops over alert configs and their temporal extents, matching
flood/drought's generic orchestration shape so shared infra logic never
has to special-case this hazard.

extract_wind_speed no longer hardcodes a 3-hour output bucket width -
it derives the bucket width from the fetched temporal extent's
lead-time spectrum, and aggregates GEFS's native cadence via a per-cell,
nodata-aware max whenever the configured interval is coarser than
GEFS_NATIVE_LEAD_TIME_STEP_HOURS.

See AB#43403
duruena and others added 18 commits July 21, 2026 10:07
…et gate

tropicalCyclone.yaml has no source_target-tagged data source yet, so the
CLI can't run real hazard logic outside --infra-only. Documents the
local-only config_reader.py relaxation needed to exercise it during dev.
Add TODO-infra notes for COUNTRY_CONFIGS and the bounding-box buffer,
drop the misleading lazy-load comment on the population raster (it was
already loaded upfront regardless), reword a stale flood-specific
docstring, and move derive_storm_centroid out of forecast.py into
extract_track.py to keep the orchestration file focused.
determine_alert -> determine_severities: the old name described a
downstream inference (is there an alert) rather than what the function
actually returns, a filtered list of per-bucket severities.

determine_spatial_extent -> clip_wind_extent_to_admin_areas: matches
what it actually does and mirrors flood's own naming for the
equivalent function; stays in determine_exposure.py rather than moving
into compute_wind_extent.py, matching flood's file split.
…xposure and raster rendering

compute_population_exposed (shared with flood) marked a pixel exposed
whenever its value was > 0, assuming nodata is always <= 0. GEFS wind
rasters use GRIB's own missing-value sentinel instead, a large positive
float, so every pixel read as exposed once any bucket cleared
MIN_SEVERITY_MS - reported as positive populationExposed across all
1648 PHL admin areas at a lowered threshold.

Two more places made the same assumption: extract_wind_speed's
conversion-factor scaling overflowed that sentinel to inf for
KNA/DMA/ATG instead of leaving nodata cells unchanged, and
raster_to_base64_png normalized against it as the raster's max,
rendering the storm footprint black and empty ocean white.

All three now compare against each raster's own nodata field instead
of assuming its sign.

Also fixes two pre-existing lint findings in extract_forecast.py
(naive datetime, zip over successive pairs) surfaced by touching this
file, and qualifies its nrw_logger/constants imports so ufmt and ruff
stop disagreeing on their sort order. exposure.py and raster.py
(infra-owned) get a noqa + TODO-infra note instead, since realigning
ufmt/ruff's config is out of scope here.
…ary local-run notes

Add the population raster to the Step 1 missing-input guard.
Add TODO-infra-remove notes.
rgba_png_to_float_array widened the whole image to uint64 before its bit math,
needing roughly 13GB of peak memory for a real country population raster
(~230-250 million pixels: PHL is 11612x19841, ETH 18001x13788) and exhausting
an 8GB machine before any hazard logic ran.

Decode in 1024-row chunks with a uint32 intermediate instead, which is wide
enough for the encoding's own documented maximum (256^4 - 1). Peak memory drops
to about 4GB and the output is byte-identical, verified by sha256 against the
previous implementation on real raster data.

Add unit tests covering the chunk boundaries, since row-chunking introduces an
off-by-one risk class the single-pass implementation did not have.
…ert extent across the forecast window

MIN_SEVERITY_MS was the pipeline's only storm-presence signal: run_forecasts
calls every country's hazard function unconditionally and wind speed is never
truly absent, so ordinary weather could raise an alert. extract_track now runs
before the alert-config loop and returns early when GEFS's own tracker has
nothing near the country.

compute_alert_extent picked only the bucket with the highest median wind speed,
dropping earlier buckets entirely: a storm reaching Cat 1 over location A at
hour 24 and Cat 4 over location B at hour 72 reported zero exposure for A. A
cyclone forecast is one real, moving storm rather than a static library of
independent scenarios, so the footprint now unions per-cell maxima across every
qualifying bucket.

derive_storm_centroid matched the peak wind bucket to a track bucket by exact
timestamp equality, but GEFS wind is natively 3-hourly while the ATCF tracker is
6-hourly (TAU 000,006,012,... in every real fixture cycle). Every odd 3-hour
bucket had no matching fix and silently fell back to averaging across the whole
window - the flat-average behaviour deliberately retired earlier. It now
brackets the peak time between the nearest fixes at or before and at or after,
interpolates between them by elapsed-time fraction, and clamps outside the
observed window.

TimeIntervalSeverity existed in both flood and tropical cyclone with different
fields, so each is renamed after its own metric:
TimeIntervalReturnPeriodSeverity and TimeIntervalWindSpeedSeverity.

Adapt to the ECMWF forecast-source dispatch added on this branch: extract_track
now takes a forecast_source argument, so the presence gate passes the country's
configured source. Its GEFS branch selected wind's 3-hour constant for track
bucketing, which reinstated the exact off-by-one-bucket centroid bug this commit
fixes - point it at the 6-hour track constant instead, and correct the docstring
that stated GEFS track was 3-hourly. Also make _parse_ecmwf_track_path build a
timezone-aware datetime like its GEFS counterpart, and qualify its logger calls
so the import stays ufmt/ruff-stable.
.lintstagedrc.js ran `uvx ufmt format` and `uvx ruff check`, which resolve a
fresh version on every invocation. Ruff 0.16.0 enabled UP017 and a stricter I001.

Both tools are project dependencies, so run them through `uv run --project data`
instead. The hook and python-knip.py now agree by construction, and the version
follows the lockfile rather than a number that has to be maintained.

Drop the five `# ruff: noqa` directives that existed only to work around that
disagreement.
PHL was switched to ForecastSource.ECMWF while the comment above COUNTRY_CONFIGS
still stated every country was on GEFS. PHL is the only seeded country, so its
local runs stopped with the wrong setting.

Document what selecting a source implies: both sources are implemented in the
hazard logic and switching a country is a config change. Record that ECMWF fixtures come from
fetch_ecmwf_tropical_cyclone_test_data.py, which was not documented anywhere.

Record the averaging-period assumption per source: the Harper conversion reads
from a 10-minute-equivalent mean, which ECMWF states it works in (ECMWF
Newsletter 164) and which is still unconfirmed for GEFS. Note alongside
MIN_SEVERITY_MS that ECMWF documents underestimating maximum wind for intense
cyclones, so an identical storm is likelier to clear the gate on GEFS.

Fetcher downloads is country-independent, so warn instead of refusing to run.
@duruena

duruena commented Jul 29, 2026

Copy link
Copy Markdown
Member

@jannisvisser question on TROPICAL_CYCLONE_CLASSIFICATION_BY_COUNTRY.PHL.severityClassLevels (119/154) these thresholds are in km/h, but the pipeline submits windSpeed severityValue in m/s, with no conversion anywhere in between. Right now, no real storm can ever clear these thresholds.

Proposal: align the thresholds to m/s rather than converting the pipeline's output to km/h.

Does that match your read, or is there a reason PHL's config was meant to be km/h specifically?

duruena and others added 5 commits July 30, 2026 09:46
… name

Concurrent storms were pooled into one alert and one attribution window, so a
distant storm's severity could bleed into a landfalling storm's, and
derive_alert_centroid's window-attribution gate was checked against the union
of every storm's tracked window instead of one storm's own - both correctness
bugs, not just missing functionality.

extract_track now groups ATCF rows by (BASIN, CY) into one StormTrack per
storm; forecast.py raises one alert per storm, each scoped only to the admin
areas its own track comes near, while still sharing a single wind extraction. event_name is now the storm's own identifier (e.g. WP24_2025), stable
across runs instead of a per-run timestamp.

GEFS only; extract_track raises NotImplementedError for ECMWF, with a note on
what ECMWF's BUFR format already offers (real storm names via longStormName).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 37 out of 38 changed files in this pull request and generated no new comments.

Suppressed comments (4)

data/pipelines/tropical_cyclone/forecast.py:134

  • This comment says extract_track parses ECMWF BUFR, but extract_track() currently raises NotImplementedError for ForecastSource.ECMWF (GEFS-only). This is misleading for anyone trying to switch COUNTRY_CONFIGS to ECMWF.
    data/pipelines/tropical_cyclone/constants.py:53
  • This comment states GEFS and ECMWF are both implemented in extract_track.py, but extract_track() currently raises NotImplementedError for ECMWF. The comment also implies switching a country to ECMWF is config-only, which isn’t true until ECMWF track extraction is implemented end-to-end.
# forecast_source selects which source's wind and track products a country is forecast from. Both
# GEFS and ECMWF are implemented in the hazard logic - extract_forecast.py and extract_track.py
# each dispatch on this value - so switching a country over is a config change, not a code change.

data/pipelines/infra/config_reader.py:66

  • hazard_type parsing now requires an exact enum value (case-sensitive). That’s necessary for camelCase values like "tropicalCyclone", but it’s a breaking change for existing configs that may have relied on case-insensitive values (e.g. "Floods"/"FLOODS"). Consider keeping backward compatibility by trying an exact parse first and falling back to .lower() only if the exact match fails.
        # Assign and validate hazard_type
        hazard_type_raw = config_raw.get("hazard_type", "")
        try:
            hazard_type = HazardType(hazard_type_raw)
        except ValueError:

data/pipelines/tropical_cyclone/TESTING_SCENARIOS.md:3

  • This doc is committed with a "TODO: delete this document" banner. If it’s intended to be temporary reviewer guidance, it’s better to either remove the TODO line or convert it into a tracked reference (e.g. AB#42859) so it doesn’t ship as lingering TODO documentation.
**TODO: delete this document once these scenarios have been tested.**

@jannisvisser jannisvisser left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It alls looks really great @duruena. Follows exactly what we talked about last week, and with lots of attention to detail and clarity in code and comments. The remaining comments are small, and are partly oriented to hand-over to Gulfaraz as well.
Functionally it comes out like this in front-end now for scenario A 👍

@gulfaraz (for full disclosure)

  • this required changing FE-code to GET events with active=false (as this was a historic event, could also be done by changing the view-timestamp)
  • I also delete from "alert-exposure-admin-area" where value = 0 in db (see another comment on this)
Image

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not clear to me what the point of this file is. I get it downloads data, but what is subsequently done with that data. Also, this is in the seed_data_management folder, which implies its output is pushed to the seed data repo. Is it? And if so, is anything subsequently fetching this from the seed data repo again? Or was this part of Jacopo's additions? (Which I specifically didn't review yet FYI)

Comment on lines +67 to +71
The pipeline always loads the most recently dated cycle folder found under `gefs_wind/` and
`gefs_track/` (folders are named `gefs.<YYYYMMDD>`). To run a specific scenario, rename every
cycle folder dated **after** the one you want, in **both** `gefs_wind/` and `gefs_track/`, by
prefixing it with `_setaside_`. This hides it from selection without deleting it. Reverse the
rename afterward to restore normal behavior.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that this whole setup is for local developing/testing only anyway, this sounds a little complicated. Instead you could have added a cmd-line flag specifying which yyyymmdd to pick. But anyway, it's fine for now.

Comment on lines +175 to +179
if not storm_tracks:
logger.info(
f"No tropical-cyclone tracked within the monitoring box for '{country}'"
)
return

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn't just return in this case. The pipeline always POSTS a forecast, just with an empty alerts array. Given the logistical context, this can also be implemented by dev though.

Comment on lines +209 to +221
# Storms this close together share admin areas, so the same population is reported as
# exposed under both events and the two figures must not be summed. Expected for storms
# within roughly a monitoring box of each other - reported, not treated as an error.
for first_storm, second_storm in find_storm_pairs_sharing_place_codes(
storm_tracks, place_codes_per_storm
):
nrw_logger.log_warning(
logger,
nrw_logger.LogTag.TROPICAL_CYCLONE_LOGIC,
f"Storms '{first_storm}' and '{second_storm}' were scoped to overlapping admin "
f"areas in '{alert_config.spatial_extent_name}'; their population-exposed figures "
f"cover some of the same people and cannot be added together",
)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, nice edge case thinking. Only a warning is good enough for now indeed. The code-block can on itself be a reminder to do more with this in the future.


- 1 alert, `eventName: "WP24_2025"`
- Centroid: `latitude: 20.503`, `longitude: 121.574`
- Median severities: `38.57`, `41.46`, `45.69` m/s (3 buckets)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just checking, I guess this is only 3 buckets because there really only 3 time-intervals above min-threshold right? (And not still limited dataset somehow).

Comment on lines +80 to +83
if parsed.lead_hour > max_lead_hour:
# GEFS provides lead hours beyond what the configured temporal extent needs (e.g. out
# to f240); not an error, just outside the requested window.
continue

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not following this completely, but I'm fine to trust you on this. In such a case, I would normally revert to assessing functionally if things work as expected. But that is difficult because we are not uploading any track output yet. Not to API of course, but also not to the DataSubmitter class.

I know nothing is set up yet for passing the track to the data_submitter. But it might be good to set up something basic for that (data_submitter.add_track_data), so that we can already add this, even though we don't upload anything to the API yet. This way we can have a little more insight yet in how the track is processed, e.g. relating to this 3h/6h discussion above. I'm guessing at this stage you would pass an array of trackpoints, with attributes lat/lon/timestamp. This could get a TODO-infra to solve better later. Plus a comment that the format might/will change based on the cone-discussion.

Given the logistical context, dev can play a role in this of course. So leaving this open for Gulfaraz to assess also.

Comment on lines +211 to +216
# Storm-center fix from the same bucket compute_alert_extent picked as peak-intensity
# (falls back to the admin-area centroid only if there are no track fixes at all - see
# docstring).
centroid = _derive_storm_centroid(
track_fixes, time_interval_severities, target_admin_areas
)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The scenario A centroid comes out here (according to https://www.gps-coordinates.net/), so in sea. So this is an exmaple of 'closest to land' then I assume right? Scenario B has a similar location in the sea, so there is no example of 'above land' centroid. It's fine for now, but would be good to have an example of this at some point.
image

Comment on lines +189 to +190
1. Confirm `gefs_wind/` and `gefs_track/` each list `gefs.20210512`, `gefs.20240929`,
`gefs.20250918`, `gefs.20260710`, `gefs.20260711`, `gefs.20260724` with no `_setaside_` prefix

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could clean up this file + the shared folder itself to only the 3 relevant dates (gefs.20210512, gefs.20240929, gefs.20250918). That will benefit also e.g. Gulfaraz. Maybe good to share the location also in the PR description.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the PR description already. @duruena can you make sure @gulfaraz and others have access to it if applicable?

# Note the gate is not equally sensitive across sources: ECMWF documents that it underestimates
# maximum wind for intense cyclones (ECMWF Newsletter 164), so the same storm is likelier to clear
# this threshold on GEFS than on ECMWF.
MIN_SEVERITY_MS = 33.0

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO-infra: we should indeed change alert-config thresholds to m/s, so 33 for low and 43 for high.

Image

Comment on lines +24 to +26
`tropicalCyclone.yaml` has no `source_target`-tagged data source yet, so a real run needs one
local-only change. Open `data/pipelines/infra/config_reader.py` and in `_parse_countries`,
change:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good. I've made a note in todo-infra task to solve this in a better way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request that affects our end users

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants