PIANO performance data parser

[lrobion]

Draft for now as I clean this up. It targets main, but we should switch to the develop branch on it's setup

This PR is part of #158 and implements a converter of PIANO performance outputs to AEIC .toml files that follow existing conventions + carry additional information not contained in .PTF files (most notably cruise performance is a function of (FL, mass, mach).

This PR implements a stub of the PianoPerformanceModel in performance/piano.py based on data stored in the .toml but does not implement the actual performance evaluation method evaluate_impl(), this is for a future PR.

PIANO output description

PIANO outputs with dummy data have been added to tests/data/piano/ and are representative of real outputs. Data is stored in 3 files, one for cruise, one for climb and one for descend. I recommend taking a look at these files first to make sense of the rest of the description here.

Climb

PIANO climb outputs assume a fixed speed schedule with PIANO-default values: CAS=250 kts when altitude < FL100, aircraft specific constant CAS for FL100 < altitude < crossover_altitude, and an aircraft specific constant Mach for altitude > crossover_altitude.

The table contains the following columns: Altitude, time, distance, cumulative fuel burn, net thrust per engine, rate of climb, drag.

This can be computed for a range of starting masses, such that there are multiple climb tables in a single climb file. However climb table headers which contain (starting mass, speed schedule...) only appear above a table when the cruise has reached its target altitude (which it does not always reach):

• The speed schedule is independent of starting mass so if there is at least one climb table header in the file, we can infer the schedule from the file directly.
• We cannot do this for starting mass as it varies from climb to clim, such that the users need to provide this data in the PianoOverrides dataclass

Cruise

PIANO computes a performance sweep at cruise as a function of (FL, mass, mach) and provides: TAS, CAS, Drag, Max. Cruise Rating, L/D, FuelFlow, SFC, Specific Air Range, Max Climb Rating / engine, Rate of climb at Max climb rating for a fixed Mach, Rate of Climb at max climb rating for a fixed CAS (+ some others, I am only storing these for now).

On top of these "parameter sweep" points, it also provides performance at notable operating points: max specific air range, 99% specific air range, maxLim. These points are appended at the end of a mach sweep: their (FL, mass, mach) coordinate may be repeated from the sweep but there is no guarantee that the values associated are the exact same as the ones from the sweep (probably some rounding in the PIANO output). Currently I overwrite this with data from the sweep but we could do the opposite.

Descent

Very similar to climb, except that they in my experience always succeed such that the descent table header is always present and can be parsed to get the speed schedule and mass, simplifying the processing (no need for PianoOverrides. PIANO also provides the altitude after which thrust is set to idle, this is also recorded in the .toml file if we ever have use for it.

PIANO.toml schema

A PIANO based .toml file looks like this (dummy numbers again):

# Performance model type (one of: legacy, bada, tasopt, piano).
model_type = "piano"

# ==============================================================================
#
#  COMMON FIELDS
#
# Fields common to all performance model types.

aircraft_name = "piano_aircraft, piano_engine"
aircraft_class = "narrow" # wide, narrow, small, freight
ISA_offset = 0
maximum_altitude_ft = 41000
maximum_payload_kg = 200
number_of_engines = 2 # Number of engines

# ------------------------------------------------------------------------------
#
# Speed data
#

[speeds.climb]
cas_low = 120
cas_high = 125
mach = 0.75
crossover_altitude_m = 10000.0

[speeds.descent]
cas_low = 120
cas_high = 125
mach = 0.75
crossover_altitude_m = 10000.0

# ------------------------------------------------------------------------------
#
# LTO data
#

[LTO_performance]
source = "EDB"
ICAO_UID = "01P11CM121" # Add UID for EDB data
rated_thrust = 11000.0

[LTO_performance.mode_data.idle]
thrust_frac = 0.07
fuel_kgs =0.01
EI_NOx = 0.01
EI_HC = 0.01
EI_CO = 0.01

[LTO_performance.mode_data.approach]
thrust_frac = 0.3
fuel_kgs = 0.01
EI_NOx = 0.01
EI_HC = 0.01
EI_CO = 0.01

[LTO_performance.mode_data.climb]
thrust_frac = 0.85
fuel_kgs = 0.01
EI_NOx = 0.01
EI_HC = 0.01
EI_CO =0.01

[LTO_performance.mode_data.takeoff]
thrust_frac = 1.0
fuel_kgs = 0.01
EI_NOx = 0.01
EI_HC = 0.01
EI_CO = 0.01


# ==============================================================================
#
#  MODEL-TYPE SPECIFIC FIELDS
#

# ------------------------------------------------------------------------------
#
# Performance table data.
#

[climb_flight_performance]
cols = [
  "fl",  # Flight levels
  "mass",  # kg
  "tas",  # m/s
  "rocd",  # m/s
  "fuel_flow",  # kg/s - REQUIRED; OUTPUT COLUMN
  "time",  # s - cumulative from start of phase
  "distance",  # m - cumulative from start of phase
  "burn",  # kg - cumulative from start of phase
  "fn_per_engine",  # N - net thrust per engine
  "drag"  # N
]

data = [
  [0.01, 0.01, 0.01, 0.01, 0.01, 0.01, 0.01, 0.01, 0.01, 0.01],
...
]

[cruise_flight_performance]
cols = [
  "fl",  # Flight levels
  "mass",  # kg
  "mach",  # Mach number
  "tas",  # m/s
  "cas",  # m/s
  "rocd",  # m/s
  "fuel_flow",  # kg/s - REQUIRED; OUTPUT COLUMN
  "drag",  # N
  "mcr_pct",  # percent of maximum cruise thrust
  "lift_to_drag",  # Lift-to-drag ratio
  "sfc",  # kg/(N.s) - specific fuel consumption
  "sar",  # m/kg - specific air range
  "mcl_avail_per_engine",  # N - maximum climb thrust available per engine
  "rocd_mcl_fix_mach",  # m/s - at maximum climb thrust, fixed Mach
  "rocd_mcl_fix_cas"  # m/s - at maximum climb thrust, fixed CAS
]

data = [
  [ 0.01,  0.01, 0.01, 0.01,0.01,  0.01, 0.01,  0.01, 0.01, 0.01, 0.01, 0.01, 0.01, 0.01],
# Repeated (N FL, M cruise masses, K Machs)
]

[descent_flight_performance]
cols = [
  "fl",  # Flight levels
  "mass",  # kg
  "tas",  # m/s
  "rocd",  # m/s
  "fuel_flow",  # kg/s - REQUIRED; OUTPUT COLUMN
  "time",  # s - cumulative from start of phase
  "distance",  # m - cumulative from start of phase
  "burn",  # kg - cumulative from start of phase
  "fn_per_engine"  # N - net thrust per engine
]

data = [
  [ 0.01, 0.01,  0.01, 0.01, 0.01, 0.01, 0.01, 0.01],
...
]

[cruise_reference_mach]
cols = [
  "fl",  # Flight levels
  "mass",  # kg
  "max_sar",  # Mach at maximum specific air range
  "sar_99",  # Mach at 99% of maximum specific air range
  "max_lim"  # Maximum limiting Mach
]

data = [
  [ 0.01,0.01, 0.01, 0.01, 0.01],
# Repeated (N FL, M cruise masses)
]

[descent_idle_thrust]
cols = [
  "mass",  # kg
  "idle_thrust_altitude"  # m - altitude below which idle thrust is used
]

data = [
  [ 0.01, 0.01],
# Repeated N descent masses
]

Code refactors

• Moved shared .toml writer functions from the legacy write code in commands/make_performance_models.py to commands/_performance_models_toml.py and only kept legacy specific and PIANO specific CLI entry points in commands/make_performance_models.py
• Split the existing PerformanceTableInput data structure into a table specific class, and a performance table class. The new TableInput dataclass (in performance/types.py) accepts empty tables and is responsible for normalizing columns, checking shape consistency and provides a column accessor. Now PerformanceTableInput inherits from it and only performs validation on the columns making sure they contain the minimum performance data required.

These refactors do not change the behavior of the existing CLI command for legacy models as tested with the reference tests/data/performance/legacy_golden.toml generated with the old code, and tests/test_make_performance_model.py which verifies that the new code leads to a byte identical file.

New code to support PIANO

Mainly in parsers/piano_reader.py and tests/.

More details to come

In progress

Code:

• Update doc
• Cleanup Claude comments...
• Evaluate test coverage

Modeling decisions:

• Get feedback on if this is the data we need / format etc...
• Simplify climb speed schedule parsing logic: assume that a PIANO climb output file will contain at least 1 valid header block describing the speed schedule (leaving PianoOverrides to only include climb masses)
• Discuss cruise mach Speed value for a PIANO which does not have a set cruise mach
• Operating empty weight handling: this field is not written to toml file because BADA assumes oew = min_mass / 1.2, whereas PIANO can provide the exact number (though it is not in the outputs we parse)

[ian-ross]

Just took a quick look at this. I know it's only a draft and you're likely to make changes, but I wanted to head off some stuff that you (or Claude) are doing that's not good.

[ian-ross]

"Parsing never fabricates rows"? I would hope not...

[lrobion]

Addressed in 2bba9c1

[ian-ross]

What is the "discriminator"?

[lrobion]

The 4th column of PIANO's cruise tables flags the row as coming from the Mach sweep (indicated by |), or a specific performance point (3 values: maxLim, maxSAR, 99%SAR).
I'll make that clearer in the comment.

I don't think I explicitly define the expected schema, so I could add that as a block comment at the top instead of piecing it together from the different expected columns.

[ian-ross]

Yeah, that would probably be good. It gives you a natural place to define the term "discriminator", which you can then use freely in the rest of the code.

[lrobion]

Addressed in 2bba9c1

[ian-ross]

Nopey-nope-nope. Don't do this. This is coding agent trash. It's copied this from the test_config.py file, which is the only place where this should be done in tests.

[lrobion]

Addressed in c0d6934

[ian-ross]

Same nopey-nope-nope here.

[lrobion]

Addressed in b7ba4d3

[ian-ross]

I dislike this very much.

[lrobion]

Addressed in b7ba4d3

[ian-ross]

I think you need to make this some sort of cached fixture. You call it in a lot of tests below, and you're only going to be adding more tests, so calling it more, which is going to get slow. But I've now looked back at what you were doing, and I think you need to organize things somewhat for testing. Don't use Click's CLI runner for this. Design for testability instead.

(This is really one of those cases where using a coding agent gets very annoying. They think they have to test all the things, and they have no taste or discretion about where to set the testing boundaries. Then they see that there is some way to work around the default test configuration and they copy it and use it in a completely inappropriate place. Instead, think about where the test boundary should be: you do not need to be running CLI commands within tests. Click works.)

[lrobion]

That makes sense. I think refactoring the code separating the Click runner from the the actual parsing/writing would help remove the default_config fixture trick and avoid running Click commands in testing.

I am considering moving the logic from commands/make_performance_models.py to a new module e.g. performance/model_writer.py and only keep Click logic + Config.load() + light arg parsing in commands/make_performance_models.py.

performance/model_writer.py would then contain the writer function and two new functions build_*_performance_model(parsed_cli_args...) -> *PerformanceModel. Then the writer can have this signature: write_performance_toml(output_file, PerformanceModel) -> None.

The point of all this is that now we can test via build_*_performance_model(parsed_cli_args...) and inspect the performance model that's outputted directly instead of having to do CLI calls to run the parsing/writing.

Does this refactoring make sense and address the issue?

@click.option(
    ...
)
@click.pass_context
def piano(...):
    # check apu
    # resolve lto
    # resolve speed schedule overrides
    performance_model: PianoPerformanceModel = build_piano_performance_model(...)
    write_performance_toml(output_file, performance_model)

[ian-ross]

Yes, that sounds much better. In general, pure functions are much easier to test (you build_*_performance_model would be pure), things that write files or do other kinds of mutations take a little more infrastructure to test (but write_performance_toml just writes a file, which is the easiest of these kinds of cases), and things that run whole external programs or try to do "clever stuff" are the worst (the test_config.py nastiness was supposed to be localized exactly to that one place where I was testing the configuration system itself — it's definitely not something that should be copied around and used in normal tests!).

[lrobion]

Sounds good, I'll implement this and ask you to take a another look then.

[ian-ross]

Oh yeah, to add to my review comments: in principle, this looks great, and it's a good first step to adding more and better performance models. We just need to wrangle the Claude.

[lrobion]

I've refactored the code such that:

• commands/make_performance_models.py only handles CLI and basic argument parsing before calling a performance model builder, and a performance model writer
• performance/model_builder.py now contains the code to create a performance model object build_*_performance_model(...) -> PerformanceModel, and a writer write_performance_model(output_path, model) -> None
• Added a schema description for each PIANO file type in performance/parsers/piano_reader.py, the placement feels a bit awkward but I was not sure where else to put it. This file is largest because it has to deal with all the PIANO specific parsing... I am not sure if I should split it up in 3 (climb, cruise, descent) but then I also need to share helper functions so it gets a little messy.
• conftest.py now has fixtures for PIANO data and LTO data, I had to use a functools.cache to get the caching working as session- or module-level scoping of the fixtures did not work with the default_config setup
• Rewrote all tests to test the builder outputs to avoid any invocation of click which is fine now that the CLI does very light work before passing it to the functions we do test. I think this setup is definitely an improvement over what I had before

On top of updating the doc, there are couple of decisions left which are more science related:

1. AEIC .toml files do not store the operating empty weight of the aircraft even though this is required for simulations. We get away with this because the LegacyPerformanceModel does the BADA assumption of take the min_tabuled_mass / 1.2 to estimated OEW. This does not work for PIANO because we don't know how BADA chooses that mass and we can do better anyway. PIANO can provide OEW directly, thought it is not in the output files we parse. Currently it can be passed as a CLI argument to aeic make-performance-model ... piano but did not make it a required argument because I was confused by this. Moving forwards I think we need to store OEW in the .toml, next to maximum_payload_kg and have the reader load it from there instead of the BADA factor.
2. The Speeds class requires a SpeedData value for its .cruise member. For tabulated data with Mach sweeps, I think this does not make sense anymore as there is no set cruise speed (unlike BADA). I went around this by allowing .cruise to be None but this feels like a weird fix to a deeper conceptual issue that the Speeds class models. I've added the option to set a cruise speed manually via a CLI argument but I do not see why we would want to use it. I'd like other people's opinions on how to work around / redesign this in the future @WyattGiroux @aditeyashukla @ShreyaSharma2023
3. Speed schedule headers for climbs are not always present in PIANO which is why there are 4 optional CLI flags to pass them manually. This is a bit annoying and if we are willing to bake in the assumption that these headers always exist, this could simplify some code. On the PIANO data end, @ShreyaSharma2023 and I could fix this by regenerating some outputs to ensure the schedules are present.

I think point 1. is the only functionality I would want decided/finalized before considering this to be done (barring code improvements). Point 2. and 3. could wait.