Fuselage layout enhancement and modification#1205
Conversation
…ECONOMY. Fixed a bug DetailedCabinLayout. Added business class to DetailedCabinLayout
…ECONOMY. added Aircraft.Fuselage.SEAT_WIDTH_BUSINESS and Aircraft.Fuselage.SEAT_WIDTH_FIRST. removed LEAPS1.
…ge.SEAT_WIDTH_FIRST
…fect fuselage layout
…t is used for transporter aircraft using FLOPS based geometry.
| else: | ||
| if design_type == AircraftTypes.BLENDED_WING_BODY: | ||
| if verbosity >= Verbosity.BRIEF: | ||
| raise UserWarning('Aircraft.Fuselage.SEAT_WIDTH_ECONOMY is not set.') |
There was a problem hiding this comment.
Does it default? If so we should let the user know what value it defaulted to.
There was a problem hiding this comment.
GASP does not have a default value. This will throw an error message. Maybe, we can take the FLOPS default (20 inches)? @cmbenne3
| num_seat_abreast_economy = 8 | ||
| if num_seat_abreast_first > 0 and num_seat_abreast_first <= 0: | ||
| if verbosity > Verbosity.BRIEF: | ||
| print('Set Aircraft.CrewPayload.Design.NUM_SEATS_ABREAST_ECONOMY = 8') |
There was a problem hiding this comment.
Very small comment - Perhaps remove hardcoded value of '8' in the print() and format with f'{num_seat_abreast_economy'} like the print statements below for consistency.
|
|
||
| self.aviary_options = AviaryValues() | ||
|
|
||
| self.aviary_options.set_val(Aircraft.CrewPayload.Design.NUM_SEATS_ABREAST_ECONOMY, 18) |
There was a problem hiding this comment.
Why don't we need this value anymore? Does it default to 18 if we don't set it?
| @@ -44,7 +44,7 @@ def compute(self, inputs, outputs): | |||
| options = self.options | |||
| verbosity = options[Settings.VERBOSITY] | |||
| seats_abreast = options[Aircraft.CrewPayload.Design.NUM_SEATS_ABREAST_ECONOMY] | |||
There was a problem hiding this comment.
I know it's not a change you've made in this PR, so perhaps out of scope here, but I think NUM_SEATS_ABREAST variables should be under Aircraft.Fuselage. not under Aircraft.CrewPayload.Design
| print('Warning: No business class passengers or cabins are included.') | ||
| if (pax_FC + pax_BC) > pax: | ||
| raise ValueError( | ||
| 'Number of first class passengers must not exceed the total number of passengers.' |
There was a problem hiding this comment.
| 'Number of first class passengers must not exceed the total number of passengers.' | |
| f'Number of first {pax_FC} and business {pax_BC} class passengers must not exceed the total number of passengers {pax}.' |
Not sure if you can include an f string value inserts in a ValueError message, but if so it might be helpful!
| ] | ||
|
|
||
| # The 200 was derived from B757 - the largest single aisle western desig. | ||
| if num_economy_class_pax > 200: |
There was a problem hiding this comment.
This comment is outside the scope of the PR, and probably one for @jkirk5 - should these if/else blocks that set the values of inputs/options be moved out of the fuselage calculation component and into preprocessors? That would clean up the component, and set the 'defaults' for sensible values elsewhere in the code...?
jkirk5
left a comment
There was a problem hiding this comment.
Some additional comments, full review WIP
| if design_type == AircraftTypes.BLENDED_WING_BODY: | ||
| if verbosity >= Verbosity.BRIEF: | ||
| raise UserWarning('Aircraft.Fuselage.SEAT_WIDTH_ECONOMY is not set.') |
There was a problem hiding this comment.
As far as I can tell there isn't a reason to raise an error if this variable is missing, I don't think we need these lines
| if verbosity >= Verbosity.BRIEF: | ||
| aviary_options.set_val(Aircraft.Fuselage.SEAT_WIDTH_FIRST, 28.0, 'inch') |
There was a problem hiding this comment.
You are only setting this default if verbosity is set to a certain level?! Maybe the verbosity line got included by accident, remove it
| if mass_method == LegacyCode.FLOPS: | ||
| if design_type == AircraftTypes.TRANSPORT: | ||
| aviary_options.set_val(Aircraft.Fuselage.SEAT_WIDTH_FIRST, 25.0, 'inch') | ||
| else: |
There was a problem hiding this comment.
The way this code is set up is there are only defaults for FLOPS transport and GASP BWB being set here? If so, add an elif mass_method is LegacyCode.GASP: instead of just an else so it still works if more mass methods are added.
| print('Warning: No business class passengers or cabins are included.') | ||
| if (pax_FC + pax_BC) > pax: | ||
| raise ValueError( | ||
| 'Number of first class passengers must not exceed the total number of passengers.' |
| def test_detailed_layout(self): | ||
| """Test DetailedCabinLayout component.""" | ||
| case_name = 'LargeSingleAisle1FLOPS' | ||
| flops_inputs = get_flops_inputs(case_name) | ||
| flops_inputs.set_val(Aircraft.Fuselage.SIMPLE_LAYOUT, False) | ||
| flops_inputs.set_val(Settings.VERBOSITY, 0) | ||
|
|
||
| # do not override these variables which are outputs from DetailedCabinLayout | ||
| flops_inputs.delete(Aircraft.Fuselage.LENGTH) | ||
| flops_inputs.delete(Aircraft.Fuselage.PASSENGER_COMPARTMENT_LENGTH) | ||
| flops_inputs.delete(Aircraft.Fuselage.MAX_WIDTH) | ||
| flops_inputs.delete(Aircraft.Fuselage.MAX_HEIGHT) | ||
|
|
||
| engines = [build_engine_deck(flops_inputs)] | ||
| preprocess_options(flops_inputs, engine_models=engines) | ||
| # get geom subsystem only | ||
| default_premission_subsystems = [get_default_premission_subsystems('FLOPS', engines)[1]] | ||
|
|
||
| prob = self.prob | ||
|
|
||
| prob.model.add_subsystem( | ||
| 'pre_mission', | ||
| CorePreMission( | ||
| aviary_options=flops_inputs, | ||
| subsystems=default_premission_subsystems, | ||
| subsystem_options={}, | ||
| ), | ||
| promotes_inputs=['*'], | ||
| promotes_outputs=['*'], | ||
| ) | ||
|
|
||
| setup_model_options(prob, flops_inputs) | ||
| prob.setup(check=False, force_alloc_complex=True) | ||
| set_aviary_initial_values(prob, flops_inputs) | ||
| prob.run_model() | ||
|
|
||
| expected_values = { | ||
| # geometry subsystem, DetailedCabinLayout component | ||
| Aircraft.Fuselage.LENGTH: 148.37731944, | ||
| Aircraft.Fuselage.PASSENGER_COMPARTMENT_LENGTH: 108.37731944, | ||
| Aircraft.Fuselage.MAX_WIDTH: 12.19, | ||
| Aircraft.Fuselage.MAX_HEIGHT: 13.09, | ||
| } | ||
|
|
||
| tol = 1e-5 | ||
| for var_name, expected in expected_values.items(): | ||
| with self.subTest(var=var_name): | ||
| assert_near_equal(prob[var_name], expected, tol) | ||
|
|
There was a problem hiding this comment.
This test is completely redundant with the tests in flops_based test_fuselage.py! We don't need it here too
React
Summary
For FLOPS based geometry, business seats are added to transporter aircraft. For GASP based geometry, business class seats are added to BWB aircraft. In both cases, users can set:
For FLOPS based models, there will be differences of fuselage layout if business class seats are included for transporter aircraft because FLOPS does not consider business seats in its layout algorithm. This message is added to the top of output .csv file.
GASP based geometry does not have a detailed layout for transporter aircraft. We do not add this feature in this PR.
Related Issues
Backwards incompatibilities
None
New Dependencies
None