Refactor

[radka-j]

Overview

Aim: we want to be able to both generate sector shapes as is currently the case (e.g., an "I" sector) and to read them from (real world) data (e.g., sector 11). To accommodate this, some changes to the current structure are needed. Specifically in the implementation of the SectorElement and SectorShape classes.

Note that SectorShape class has 3 key attributes that are used by SectorElement:

• polygon
• fixes
• routes

The rest of the class is methods for generating these given some input parameters. The SectorElement class takes in a shape and generates the GeoJSON.

TO DO:

1. Refactor SectorElement:

• currently the class takes in sector type (e.g., "I") and generates the shape class given the type -- it should not be generating anything, it should either take in the shape class or just the relevant key attributes (as listed above)
• the deserialise() method similarly re-generates a sector using default values given a type rather than truly parsing all the data from the GeoJSON, this is incorrect behaviour - read in all the data instead
• move origin, upper/lower FL limits to shape class

2. Simplify/refactor base SectorShape:

• remove constructor
• remove params specific to the generated sector shapes (e.g., length, airway etc.)
• remove default fix names etc. and move to relevant shape class (e.g., IShape)
• clean up code (e.g., methods for computing fix and route locations are only part of the relevant generating shapes

3. Implement new Shape that reads in sector data:

• decide where best to hold the data to read
• input param can just be a sector name
• include methods to parse the polygon as well as fixes and routes associated with a given sector from the data

4. Update tests

• Sector Element
• SectorShape
• All shape classes (I, Y, X)
• New tests for new class

5. Other

• Update script generating sector files
• Update documentation

6. Scenario generation

• Update scenario classes
• Update tests
• Update scenario generation

Questions

Do we want to simplify the generated GeoJSON? Are we actually going to use the GeoJSON?

Notes

Some original notes on this also here

[FreddyWordingham]

As some waypoints (fixes) lie in multiple sectors, perhaps we could have a global dict of all UK waypoints, and then individual sectors only store the keys to waypoints in their vicinity to stop data duplication. If waypoint 'Spirit' is moved in it's own airspace for example, this change needs to be reflected in the adjacent Sector-I airspace.

We've used Shapely before to define sector geometry, but a simple list of 2d points, and then the winding number can be used to compute if a point is inside. Then we don't need to rely on shapely as a dependency- and (de)serialisation is then also little more transparent

Does sector name need to be defined within itself? If there's more than one sector then the parent class can hold a dict of sectors, and name them that way. If there's a single sector then it doesn't need a name (at least within itself). If it's intended for storing in a file, then the filename should be the name of the airspace.

Are routes bound to an airspace - or an airplane? I guess airspaces will have common routes, but if a plane is rerouted then this will differ to the common routes(?). Perhaps the route info (stored as a mutable list of waypoint key names) should be held only by the aircraft?

[radka-j]

Yes, there's definitely some thinking to be done about how to best store and structure all the data. And this will become especially important once we start thinking about a full airspace rather than individual sectors as we are doing now. I think some of the format (e.g., the full geoJSON) was a requirement for the original NATS simulator - which is no longer under development. I am not sure what in the current structure is a requirement for maintaining BlueSky compatibility though. So that is something that will also need reviewing and then we might find we can scale down/simplify quite a bit of the current structure. I think that restructuring data might be easier to tackle as a next, separate issue though. It also naturally belongs with thinking about the full airspace definition (rather than individual sectors as we do now) where the scenario you describe arises.

Personally I like shapely for some of the data manipulations but you are right that perhaps it is not necessary. It might be easier for me to comment on this after this refactor, once I have more of a feel for how we are using it. I'll keep it in mind!

Currently the sector name relates to the real world sector names, which is also how they are stored in the data files. So the name here is because a user might, for example, want to load Sector 11 (which is a specific bit of UK airspace), and the name is the only way we can specify which data to load. Of course if we end up storing a file per sector (rather than reading the data each time), then yes you are right the name could just be the filename.

As far as I understand, each sector has permitted routes through it. Each plane has a flight plan (which will be along one of the routes through the sector). A plane cannot fly on a route that is not pre-defined i.e., not allowed in the sector (they tend to refer to them as highways in the sky). So you can't just pick any random combination of waypoints in the sector and fly a plane through it (at least under current operations). So at least for now you need both - routes in a sector and a plane's flight plan. This is a real world constraint but also a BlueSky constraint.

[FreddyWordingham]

Makes sense to me! Thanks Radka

[helendduncan]

Just to add to this, perhaps it is unnecessary... But on Thursday there was also discussion about definitions of sectors as we understand them (i.e. sector 11) but also "airspaces", which are the 'target' sector and the surrounding ones that ATCOs may be able to see and/or instruct at the same time. With that in mind would routes then belong to these airspaces and not sectors themselves.
There was also discussion about if scenario generation should remain in Aviary or if it should move to the simulator - specifically thinking about changing 'static' flight paths in aviary to more dynamic ones in simulator/phoenix with distributions for take-off for example

[radka-j]

You are both totally right that quite a few things that belong to sectors now should really be part of an airspace definition and the sectors should just reference those somehow (e.g, only store keys to waypoints). Those are all really useful comments, thank you both!

Interesting point about scenario generation! Being part of the simulator makes sense. The only reason for having scenario generation here is to maintain compatibility with BlueSky. As now, we can have some scenario format that suits the simulators and include a parser which can translate that format into a scenario file that can be loaded into BlueSky. With the full acknowledgement that we might build to scenarios that are too complex for BlueSky. So that might just be something to consider within this discussion.

Within aviary, we also considered scenarios that are generated according to some distribution for take-off. For BlueSky, we still need to generate a single, "static" scenario file given this stochastic/dynamic formulation. Because that is how scenarios are played in BlueSky. But the underlying generating class is dynamic and the simulators could just import that rather than a file. So the two things are not mutually exclusive (if that makes sense).

[tjdodwell]

Instead of current defintition of route I wonder if it would be helpful to have a connectivity matrix for a sector. So like a graph representation.

This is what I am using to get round some current issues in phoenix, see issue https://github.com/project-bluebird/phoenix/issues/29

[radka-j]

Is that issue because the route definition for an aircraft (in the scenario json file) is by fix index rather than fix name?

I am quite agnostic on route definition and happy to go with whatever makes it easier. For now, we can also just expand the sector class and add a method which returns the routes as a graph.

To give some context, as you know we currently have a a set list of routes through a sector where each route is a pre-defined list of fixes (e.g., you have 4 routes through the X sector). In the past, when generating a random scenario we would then just randomly choose one of the available routes for each aircraft (for example). This is also consistent with how routes are defined in the NATS data (a list of waypoints). But again, this can easily be transformed into another preferable format.

[radka-j]

Some of this work has been addressed in #48 which refactored the code to allow for loading real world sectors. The remaining outstanding work has been separated out in issues #51 , #52 and #53 to make it easier to see what is left to do.