Skip to content

Conversation

@jkirk5
Copy link
Contributor

@jkirk5 jkirk5 commented Dec 9, 2024

Summary

Fixes a bug in the crew & payload preprocessor where providing a passenger count of 0 for a single mission was confused with not providing that particular passenger count variable at all, leading to overwriting mission pax count with design pax count.

Related Issues

Backwards incompatibilities

None

New Dependencies

None

@ehariton ehariton self-requested a review December 10, 2024 15:33
@ehariton ehariton assigned ehariton and unassigned ehariton Dec 10, 2024
Copy link
Contributor

@ehariton ehariton left a comment

Choose a reason for hiding this comment

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

test_ben_FwFm.py fails on this PR.

File "/mnt/c/OMDAO/om-Aviary/aviary/validation_cases/benchmark_tests/test_bench_FwFm.py", line 397, in test_bench_FwFm_SNOPT
    compare_against_expected_values(prob, self.expected_dict)
  File "/mnt/c/OMDAO/om-Aviary/aviary/validation_cases/benchmark_utils.py", line 48, in compare_against_expected_values
    assert_near_equal(masses[-1], expected_masses[-1], tolerance=rtol)
  File "/mnt/c/OMDAO/openmdao/openmdao/utils/assert_utils.py", line 583, in assert_near_equal
    raise ValueError('actual %s, desired %s, %s error %s, tolerance %s'
ValueError: actual [43067.40134181], desired [62697.71513264], rel error 0.3130945641208417, tolerance 0.02

This code detects:
pax_provided True
design_pax_provided True

even though only Design information is provided in aircraft_for_bench_FwFm.csv

Examining aviary_options what we find is that 'aircraft:crew_and_payload:num_business_class': (0, 'unitless'), has been set before getting to the following code lines in preprocessor.py:

    for key in pax_keys:
        if key in aviary_options:
            # mark that the user provided any information on mission passenger count
            pax_provided = True
        else:
            # default all non-provided passenger info to 0
            aviary_options.set_val(key, 0)

@jkirk5
Copy link
Contributor Author

jkirk5 commented Dec 10, 2024

I will need to update docs once I have tests working

@jkirk5
Copy link
Contributor Author

jkirk5 commented Dec 27, 2024

Examining aviary_options what we find is that 'aircraft:crew_and_payload:num_business_class': (0, 'unitless'), has been set before getting to the following code lines in preprocessor.py:

It appears it is an issue stemming from the way load_vehicle() works, which is to grab every single variable from the metadata with a default and apply it to the aviary inputs. I think this is really bad because it has the potential to accidentally set default values as overrides which 100% isn't intended behavior. It also is really overkill, we only need to set values for inputs the user wants - every other default should come directly from the metadata when adding component inputs/outputs.

This PR may need to stay on hold until we clean up how defaults work in the rest of Aviary, this approach should work with the new paradigm (all defaults come from metadata rather than using user inputs as a vehicle to get those values into the problem)

"- **Maximum Fuel Point**: The maximum fuel the aircraft can hold with additional payload up to the aircraft's max takeoff weight. \n",
"- **Ferry Range Point**: The maximum fuel the aircraft can carry with zero payload to achieve maximum possible range. \n",
"- **Maximum Payload Point (\"Design Range\")**: The range the aircraft can fly with maximum payload mass and fuel mass up to the maximum gross mass. This range is typically the design point of the aircraft (the \"sizing\" mission).\n",
"- **Maximum Fuel Point (\"Max Economic Range\")**: The range the aircraft can fly at maximum fuel capacity and payload mass up to maximum gross mass. This is the furthest range the aircraft can fly at its maximum gross mass.\n",
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought this was called "max fuel" point not "max economic range" ? The wording here doesn't make me feel great and I mis interpreted it the first time I read it. I might go for something like:
The range the aircraft can fly at where the fuel tanks are completely full and payload mass is added until the aircraft reaches it's maximum gross mass.

Copy link
Contributor Author

@jkirk5 jkirk5 Dec 12, 2025

Choose a reason for hiding this comment

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

This change is actually already in main from the off-design overhaul, but I'll comment here.

I'm not sure I follow, the point is still directly named "Maximum Fuel Point" here. Each point has a name, and mentions the particular range that point corresponds to (e.g. flying the max payload point falls out design range) as additional information. The only other name I've ever heard to describe this range is "harmonic range", which to me is super unhelpful. I chose "Max Economic Range" based on the fact off-design missions that describe actual operating cases are often referred to as "economic missions". "Max Economic Range" is meant to refer to the maximum range of the aircraft at full utilization (and therefore maximizing profitability at that range). I'm open to a better name for this specific range since there doesn't seem to be a good official one.

And the sentence I have seems to me to say the exact thing you suggested? "fuel tanks completely full" == "at maximum fuel capacity", "payload mass added until reaches maximum gross mass" == "payload mass up to maximum gross mass"

@jkirk5 jkirk5 requested a review from ehariton December 12, 2025 22:00
mass_method == LegacyCode.FLOPS
and equations_of_motion is EquationsOfMotion.HEIGHT_ENERGY
):
if equations_of_motion is EquationsOfMotion.HEIGHT_ENERGY:
Copy link
Contributor

Choose a reason for hiding this comment

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

@Kenneth-T-Moore, considering the simplification here we may want to push all the below into the EOM builder.

@jkirk5 jkirk5 added this pull request to the merge queue Dec 15, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 15, 2025
Kenneth-T-Moore and others added 3 commits December 17, 2025 23:40
…rocessor that eneded up inserting the num_first class/business/tourist options into a GASP aviary_inputs, which broke some logic on the subsequent preprocess run.
Fixed some failures due to a preprocessor bug.
@Kenneth-T-Moore Kenneth-T-Moore added this pull request to the merge queue Dec 18, 2025
Merged via the queue into OpenMDAO:main with commit ce9f1c3 Dec 18, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants