Feature/sector agnostic scenario managers by nbarlowATI · Pull Request #71 · project-bluebird/BluebirdATC

nbarlowATI · 2026-05-29T14:21:40Z

Changes to decouple airspace loading from scenario managers, meaning that any of the artificial airspaces can be used with any of the "Two Aircraft", "Regular", "Infinite" etc scenario managers.

Philosophy is that the scenario_category would be something like "Two Aircraft", "Infinite" etc. and the scenario_name would be the sector ("Xplus-Sector", "I-Sector", "Springfield" etc.). (Note that "Springfield" remains as a scenario_category, with the list of springfield scenarios available, unchanged.)

Added airspace_generator/airspace_loader.py allowing the scenario managers to load airspace, routes, and default sector_name (to be used in Coordinations) for any of I, Y, X, Xplus, TwoSector, and Springfield.
- Scenario Managers now call AirspaceLoader.load(scenario_name) rather than having match/case blocks within their code.
Renamed "Tactical" scenario manager to "Custom", to better reflect its purpose.
Added functionality in Custom scenario manager to allow people to fully customize (deterministically) the scenario - adding fully specified aircraft and coordinations if they want to, and gave an example of doing this in the Intro-Part3.ipynb notebook.
Factored out some functionality that was used in more than one scenario manager (specifically code to laterally offset aircraft spawn points, and the code to actually create Aircraft and Coordinations) into utility/scenario_manager_utils.py.
Attempted to standardise things in Two Aircraft, Regular, Custom and Infinite scenario managers, such that they are all fully configurable via the setup method, all of them use np.random.default_rng for any random number generation, and the random seed can be set and used consistently.
Updated notebooks, docs, and a couple of places in bluebird-gymnasium to reflect the Tactical->Custom change.
Some changes to harmonise some code in sector i/x/xplus/y envs in gymnasium, particularly with regard to random seed treatment, and some common code in setting airspace metadata into envs/base.py.

… out some common code

…c-scenario-managers

georgedeath

Thanks for the substantial cleanup here. I found two functional issues that look worth fixing before merge, both around preserving explicit user intent in the new shared paths.

georgedeath · 2026-05-29T14:56:34Z

Some constructor parameters are probably intentionally lower-level rather than public setup() API, so I do not think every constructor argument needs to be exposed. But from the PR description it is a little hard to tell which options are intentionally hidden and which are expected user-facing configuration. For example, Infinite.__init__ has behavior-shaping options such as automatic_outcomm, aircraft_on_route, spawn_distance_behind_fix, max_spawn_attempts, min_spawn_delta, and start_time, while Infinite.setup() exposes only part of that surface.

There is also a smaller naming consistency issue in the gym env wrappers: SectorIEnv and SectorXEnv now use category "Custom", but SectorYEnv still uses "Artificial" / "Y-Sector-Custom-Scenario", and SectorXPlusEnv still uses "Artificial". That seems to undercut the stated Tactical -> Custom cleanup even if it does not break runtime behavior.

…e is None rather than general truthiness in custom.py

nbarlowATI · 2026-06-03T10:56:01Z

Thanks George - all those comments very valid+useful, and have been addressed now.

AlvaroSierra

Not quite a full review, but here are some initial comments on the PR.

AlvaroSierra · 2026-05-29T15:22:23Z

Do all these notebooks always need to be run by hand if something changes? If yes should we add CI (similarly to the HMI) which checks this has been done. Otherwise, we will eventually forget (I deffinetly will 🤣).

The notebooks normally won't need to be re-run and saved after changes to the code - the changes to notebooks in this PR are just because the scenario name and categories have changed (and an automatic CI step wouldn't be able to do that for us).
We can have CI running the notebooks to test that they aren't broken though - I have added that, but it is quite slow, so for now perhaps it should only run for PRs into main...

AlvaroSierra · 2026-06-12T10:47:05Z

+    if category in ["Two Aircraft", "Regular", "Custom", "Infinite"]:
+        return ["I-Sector", "X-Sector", "Xplus-Sector", "Y-Sector", "Two Sector", "Springfield"]


Should this list live in the AirspaceLoader type as a static method, such that it lives in the same place?

AlvaroSierra · 2026-06-12T10:57:24Z

+    # If we start or end outside the sector, use the `airspace.route_boundary_fixes` method
+    boundary_fixes = list(airspace.route_boundary_fixes(route).places.keys())


This function returns the boundary of the airspace, not a specific sector, which although valid in default scenarios, it is not generalisable.

Also, a simple performance improvements:

Suggested change

# If we start or end outside the sector, use the `airspace.route_boundary_fixes` method

boundary_fixes = list(airspace.route_boundary_fixes(route).places.keys())

# If we start or end outside the sector, use the `airspace.route_boundary_fixes` method

boundary_fixes = set(airspace.route_boundary_fixes(route).places.keys())

There are a couple of things we could do here, with different levels of upstream change.

We could move the route_boundary_fixes method from Airspace to Sector, which would seem to make sense to me. However, Sector doesn't currently have any dependency on Fixes, so I'm a bit reluctant to introduce it now.

We could use the logic in Airspace.route_boundary_fixes directly here.

My current thinking is to do the simple part of this (i.e. see if fixes are actually on the boundary) here in this code, as it is only a couple of lines. The other part of Airspace.route_boundary_fixes, that looks for fixes near a boundary, has a comment in the code implying it needs a bit of work anyway.

AlvaroSierra · 2026-06-12T10:58:25Z

+    if not entry_fix:
+        # start from the beginning of the route
+        for fix in route.filed:
+            if fix in boundary_fixes:
+                entry_fix = fix
+                break
+        # if no match, revert to first fix in route
+        entry_fix = entry_fix if entry_fix is not None else route.filed[0]
+    if not exit_fix:
+        # start from the end of the route
+        for fix in reversed(route.filed):
+            if fix in boundary_fixes:
+                exit_fix = fix
+                break


Both of these could resolve to having the same entry and exit fix, should we fall back if this is the case?

Right, good point, I now check in the reverse loop that the first boundary fix encountered is not the entry fix, before assigning it as the exit fix.

AlvaroSierra · 2026-06-12T10:59:32Z

+            if fix in boundary_fixes:
+                entry_fix = fix
+                break
+        # if no match, revert to first fix in route


If no match, should we fail rather than create an unintended scenario?

I don't think so - taking first/last fix in the route is what we currently have in main/dev, so as a fallback I think it's not so bad. It is an upgrade (useful, according to Ese) to use the actual boundary fixes if we find them, but if we end up in the fallback situation we are no worse off than now.

AlvaroSierra · 2026-06-12T10:59:37Z

+                exit_fix = fix
+                break
+        # if no match, use last fix in route
+        exit_fix = exit_fix if exit_fix is not None else route.filed[-1]


If no match, should we fail rather than create an unintended scenario?

AlvaroSierra · 2026-06-12T11:00:49Z

+    """
+    # If airspace is not provided, fall back on first and last fixes on route
+    # for the entry and exit fixes
+    if not airspace:


In what cases would the airspace not be provided?

Hmm... good question... The airspace is only needed in this function for finding the entry and exit fixes, so I originally added it as an optional argument, without which we'd fall back to the first/last fixes on route (the current situation). But it's true that this function is unlikely to be called directly by a user, rather than by a scenario manager, so they should always have an airspace. I will make it a mandatory argument, and remove this "if" block.

AlvaroSierra · 2026-06-12T11:03:33Z

+def create_aircraft_with_coordinations(
+    callsign: str,
+    pos: Pos3D,
+    heading: float,
+    speed: float,
+    route: Route,
+    sector_name: str,
+    entry_fl: float,
+    exit_fl: float,
+    on_route: bool = False,
+    airspace: Airspace | None = None,
+    prev_sector: str = "background",
+    next_sector: str = "background",
+    typeof_aircraft: type[TAircraft] = Aircraft,


I'm not a huge fan of having these helper functions dotted around the code, as they become something extra to maintain, is there a specific reason why this:

shouldn't be a separate constructor in the types which it is building

needs to be abstracted away from the callers

I agree having this type of utility function is a bit messy, but not sure what the alternative is. I don't how this could be a separate constructor, given that it's returning different types (an Aircraft and two Coordinations). And the reason to abstract it away from the callers is that it's called by several scenario managers and we shouldn't duplicate the same code in all of them.
The alternative I can think of is to have it as a method in a base ScenarioManager class, so that the concrete scenario managers get it via inheritance, but then not all scenario managers (e.g. real data) would use it, so then we would maybe add another layer of inheritance (an abstract ArtificialScenarioManager or something), which would add complication.

I'm open to suggestions, but this is the least worst option I've come up with.

AlvaroSierra · 2026-06-12T11:04:58Z

+        to_sector=sector_name,
+        fl=entry_fl,
+        fix=entry_fix,
+        direction="Horizontal",


Should all ordinations be horizontal?

yep, fair point, I added this as an optional argument with default "Horizontal".

AlvaroSierra · 2026-06-12T11:12:18Z

+                    save_log_to_file=save_log_to_file,
+                    simulated_sectors=simulated_sectors,
+                )
+            case "Custom":


Should we change the naming, as from the scenario name and category, it is not customisable.

Hmm... true.... but I would be in favour of still calling it "Custom" so that people can still know to look at the Custom scenario manager if they want to figure out what's going on.

…c-scenario-managers

nbarlowATI added 9 commits May 21, 2026 10:29

WIP sector agnostic scenario manager

991f61b

remove obsolete tactical.py

2a89730

update gymnasium after tactical->custom change

8e49dae

merge with dev

80da04e

harmonise treatment of random seeds across gymnasium envs, and factor…

e828f25

… out some common code

harmonise treatment of random seeds across gymnasium envs, and factor…

b49377f

… out some common code

updates to examples for new scenario category/names

22b8e30

update view of Springfield in intro-part 2 notebook

500aae6

Merge remote-tracking branch 'origin/dev' into feature/sector-agnosti…

c76eb09

…c-scenario-managers

nbarlowATI requested a review from a team May 29, 2026 14:21

nbarlowATI changed the base branch from main to dev May 29, 2026 14:22

nbarlowATI added 2 commits May 29, 2026 15:30

fix import order to satisfy ruff

ca4a954

ruff format

b92c4bf

georgedeath requested changes May 29, 2026

View reviewed changes

Comment thread bluebird-gymnasium/bluebird_gymnasium/envs/base.py Outdated

Comment thread bluebird-dt/bluebird_dt/scenario_manager/custom.py Outdated

nbarlowATI and others added 5 commits June 1, 2026 16:17

consistency in naming categories and scenarios

d35a08b

don't overwrite airspace origin if already set, and check for on_rout…

2360ea6

…e is None rather than general truthiness in custom.py

expose scenario manager constructor args through setup

d966a09

Merge branch 'dev' into feature/sector-agnostic-scenario-managers

113b31b

remove unused imports

3aea717

nbarlowATI requested a review from georgedeath June 3, 2026 10:56

AlvaroSierra reviewed Jun 12, 2026

View reviewed changes

nbarlowATI and others added 4 commits June 15, 2026 10:22

Merge branch 'dev' into feature/sector-agnostic-scenario-managers

6e30f29

Fix scenario/category name in radar notebook

514866d

updates and fixes from code review

2252bcf

Merge remote-tracking branch 'origin/dev' into feature/sector-agnosti…

d28eb4c

…c-scenario-managers

		if category in ["Two Aircraft", "Regular", "Custom", "Infinite"]:
		return ["I-Sector", "X-Sector", "Xplus-Sector", "Y-Sector", "Two Sector", "Springfield"]

		# If we start or end outside the sector, use the `airspace.route_boundary_fixes` method
		boundary_fixes = list(airspace.route_boundary_fixes(route).places.keys())

Uh oh!

Conversation

nbarlowATI commented May 29, 2026

Uh oh!

georgedeath left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

georgedeath commented May 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nbarlowATI commented Jun 3, 2026

Uh oh!

AlvaroSierra left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

georgedeath commented May 29, 2026 •

edited

Loading