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

Issue 32 Resolution - Range to Distance switch #73

Merged
merged 13 commits into from
Jan 18, 2024

Conversation

ehariton
Copy link
Contributor

@ehariton ehariton commented Jan 16, 2024

Summary

Dynamic.Mission.RANGE was removed and replaced with Dynamic.Mission.DISTANCE.

To implement this, some additional names were replaced:

  • initial_range -> initial_distance
  • states:range -> states:distance
  • _Dynamic.Mission was removed from all files and longer but easier to find names were introduced.
  • Imported Dynamic to test_FLOPS_based_sizing_N3CC.py to enable it to pass tests
  • "distance_rate", Dynamic.Mission.RANGE_RATE -> Dynamic.Mission.DISTANCE_RATE
  • .RANGE_RATE - > .DISTANCE_RATE
  • (note there is still a file called range_rate which has a RangeRate() function which should eventually be renamed to DistanceRate()
  • "range", 'range', "distance", 'distance', Dynamic.Mission.RANGE -> Dynamic.Mission.DISTANCE
  • Dynamic.Mission.RANGE, Dynamic.Mission.RANGE_RATE, RANGE, & RANGE_RATE were removed from variables.py
  • Old names of range_rate and range were retained in the comments in variable_meta_data.py

Related Issues

Backwards incompatibilities

None

New Dependencies

None

@johnjasa johnjasa self-requested a review January 17, 2024 05:12
Copy link
Member

@johnjasa johnjasa left a comment

Choose a reason for hiding this comment

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

Thanks for the detail-oriented work here, Eliot! I dig it.

@jdgratz10 jdgratz10 assigned jdgratz10 and unassigned jdgratz10 Jan 17, 2024
" \"takeoff_vel_con=takeoff_vel-climb_start_vel\",\n",
" \"takeoff_alt_con=takeoff_alt-climb_start_alt\"\n",
" ],\n",
" takeoff_mass_con={'units': 'lbm'}, takeoff_mass={'units': 'lbm'},\n",
" climb_start_mass={'units': 'lbm'},\n",
" takeoff_range_con={'units': 'ft'}, takeoff_range={'units': 'ft'},\n",
" takeoff_distance_con={'units': 'ft'}, takeoff_range={'units': 'ft'},\n",
" climb_start_range={'units': 'ft'},\n",
Copy link
Member

Choose a reason for hiding this comment

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

I noticed there is a "climb_start_range" that should probably also be changed to distance.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

for sure there are a bunch of other references to range in the code. I hesitate to do a global find/replace as I'm sure that will brick things. There are around 21 instances of climb_start_range I'll do a quick find/replace on those and see if all the tests are still working afterwards.

from aviary.variable_info.variables import Mission

Dynamic = _Dynamic.Mission
from aviary.variable_info.variables import Dynamic, Mission
Copy link
Member

Choose a reason for hiding this comment

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

All of these detailed landing phases have a "max_range" which should become "max_distance".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

John was hoping to name distance_max as range I think.
we should never be using a naming scheme with max at the start.

from aviary.variable_info.variables import Mission

Dynamic = _Dynamic.Mission
from aviary.variable_info.variables import Dynamic, Mission
Copy link
Member

Choose a reason for hiding this comment

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

All of these detailed takeoff phases have a "max_range" which should become "max_distance".

Copy link
Member

@Kenneth-T-Moore Kenneth-T-Moore left a comment

Choose a reason for hiding this comment

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

This looked pretty good. Just noted a few more variables with "range" in the name that need to be changed over to distance.

@ehariton
Copy link
Contributor Author

Made updates based on Ken's comments, tests passing locally, pushing up.

@johnjasa johnjasa added this pull request to the merge queue Jan 18, 2024
Merged via the queue into OpenMDAO:main with commit 8a6a63f Jan 18, 2024
6 checks passed
@johnjasa johnjasa mentioned this pull request Feb 7, 2024
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.

Name Duplication: Dynamic.Mission.DISTANCE & Dynamic.Mission.RANGE
4 participants