Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

phase_info checks and documentation #159

Open
johnjasa opened this issue Feb 23, 2024 · 7 comments · May be fixed by #608
Open

phase_info checks and documentation #159

johnjasa opened this issue Feb 23, 2024 · 7 comments · May be fixed by #608
Assignees
Labels
code cleanup Code cleanup, refactoring, or similar helpful reorganization enhancement New feature or request usability UI/UX or API improvements

Comments

@johnjasa
Copy link
Member

Desired capability or behavior.

Currently check_phase_info has some checks for keys, usually that they are required. But we need to more formally define which keys are required or optional and check for them accordingly. We also want to have a warning or error if a user introduces a key that is not recognized or used by Aviary.

Is your feature request related to a problem? Please describe.

No response

Associated Bug Report

No response

@johnjasa johnjasa added enhancement New feature or request code cleanup Code cleanup, refactoring, or similar helpful reorganization labels Feb 23, 2024
@ehariton
Copy link
Contributor

ehariton commented Feb 23, 2024

For Reserve Phases, or phases with target_time = True, we need much fewer inputs than is currently listed in check_phase_info|common_entries.

Items like fix_duration, duration_bounds, and initial_guesses|times can be safely ignored in these cases.

Methods_for_level2 will have some checks for setting these values automatically once PR#492 is pulled in:
Target_time verification for all phases

  • Checks to make sure target_time is positive,
  • duration_bounds and initial_guesses for time have not been set,
  • Then sets duration_bounds, initial_guesses, and fixed_duration

@ehariton
Copy link
Contributor

Additionally, tests in the examples folder will need to be revised once this work is completed to remove the extra inputs:
run_reserve_mission_cruise_fixedrange
run_reserve_mission_cruise_fixedtime
run_reserve_mission_multiphase_fixedrange
run_reserve_mission_multiphase_fixedtime
run_reserve_mission_multiphase_time_and_range

assuming that all of those make it into the final release.

@jkirk5 jkirk5 added the usability UI/UX or API improvements label Jun 26, 2024
@ehariton ehariton changed the title Determine how best to ensure there are reasonable checks for phase_info phase_info checks and documentation Oct 9, 2024
@ehariton
Copy link
Contributor

ehariton commented Oct 9, 2024

Recent experiences with new users modifying phase_info revealed a lack of documentation for how the current functions really work.

initial_bounds - the is the absolute start time of the phase, this is in absolute time.
duration_bounds - this is the delta time that the phase is allowed to take,
initial_guesses_time - do we even need to include this? These guesses are in [absolute, relative] time, do these get set automatically if not provided?

unclear on what "input_initial" means
fixed_initial = you have "pinned the first value of your state"
input_initial = you are doing some external connections to find out what that first state value is

automatic calculations that should be happening: for your first phase, if you include takeoff phase, then you need fixed_initial = false, input initial=true, and this should be calculated automatically

@ehariton
Copy link
Contributor

Currently, modifying the phase_info is one of the easiest ways to break an optimization. I believe that in the long-term, we should limit interactions with phase_info to graphical-based tools for level1 and level2 users.

Many of the descriptions of the elements within phase_info are inaccurate and inconsistent.

  • fix_initial and constraint_final - these should both used a single word, and they do not say that they only apply to mach and altitude states
  • input_initial, only applies to distance and mass states (not mach and altitude)
  • items are not organized systematically: 'optimize_mach' vs. 'mach_bounds' vs. fix_initial - all of these should start with what they are modifying and end with the option they are modifying i.e. mach_optimize, mach_bounds, mach_initial_fix, mach_initial_value
  • initial_bounds only deals with time and no other states and is in absolute time of the whole mission
  • duration_bounds should be time_final_bounds in relative time from the start of the phase
  • we are using the phrases range and distance where distance is relative range and range is absolute range, this could be changed to range_phase vs. range_mission

A re-write of phase-info elements requires a return to the fundamental attributes of a states and controls
Each state has the following attributes:

  • initial value may be one of: fixed (cannot move ever), input (is connected to some other external calculation), bounded (optimizer can move in a specified range), free (optimizer can move it to any value) and initial guess for state
  • values that occur during the phase must be of: fixed, bounded, free and initial guess for state
  • final values must be one of: fixed, bounded, free, and initial guess for state

There are other elements of the phase info that deal with selectively instantiating subsystems ie. aerodynamics, controls, solvers, and anything else that must be created in a phse-specific manner i.e. happens during one part of the mission but no where else.

@ehariton
Copy link
Contributor

Some checks on phase_info exist in methods_for_level2 and some exist in check_phase_info, these should be consolidated to all exist in check_phase_info.

@ehariton
Copy link
Contributor

ehariton commented Dec 4, 2024

pre and post mission are not being checked for valid keys. This has caused incorrect keys like "constrain_range": True, to be retained in the examples even though it has no effect.
graphical_input.py is still creating 'constrain_range'

@ehariton
Copy link
Contributor

ehariton commented Dec 4, 2024

polynomial_control_order is not in check_phase_info or methods_for_level2. I believe it is required because it's used in flight_phase_builder, but it's not throwing an error (which it should if it's not on the required list).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code cleanup Code cleanup, refactoring, or similar helpful reorganization enhancement New feature or request usability UI/UX or API improvements
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants