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

Rename TAS to velocity in GASP-based phases #117

Merged
merged 17 commits into from
Feb 6, 2024

Conversation

johnjasa
Copy link
Member

@johnjasa johnjasa commented Jan 29, 2024

Summary

Renamed TAS to velocity in GASP-based phases.
Also refactored some time settings and phase instantiation so now height_energy and two_dof share more code.

Related Issues

Backwards incompatibilities

None

New Dependencies

None

Copy link
Contributor

@crecine crecine left a comment

Choose a reason for hiding this comment

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

A couple of minor questions/comments (particularly about add_flight_conditions)

@ehariton ehariton self-requested a review January 31, 2024 21:30
@@ -32,14 +32,16 @@ def setup(self):
val=np.ones(nn),
desc=Dynamic.Mission.DRAG,
units="lbf")
self.add_input("TAS", val=np.ones(nn), desc="true air speed", units="ft/s")
self.add_input(Dynamic.Mission.VELOCITY, val=np.ones(nn),
desc="true air speed", units="ft/s")
Copy link
Contributor

Choose a reason for hiding this comment

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

desc should be updated to Velocity for this and VELOCITY_RATE line 44

@@ -153,8 +153,8 @@ def setup(self):
promotes_inputs=[
(Dynamic.Mission.SPECIFIC_ENERGY_RATE,
Dynamic.Mission.SPECIFIC_ENERGY_RATE_EXCESS),
(Dynamic.Mission.VELOCITY_RATE, "TAS_rate"),
(Dynamic.Mission.VELOCITY, "TAS")],
(Dynamic.Mission.VELOCITY_RATE, Dynamic.Mission.VELOCITY_RATE),
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't need the aliasing here :)

@@ -56,7 +57,7 @@ def setup(self):
add_aviary_input(self, Dynamic.Mission.ALTITUDE, val=np.ones(nn), units="ft")
add_aviary_input(self, Dynamic.Mission.DISTANCE, val=np.ones(nn), units="ft")

self.add_output("TAS_rate", val=np.ones(nn), desc="TAS rate", units="ft/s**2",
self.add_output(Dynamic.Mission.VELOCITY_RATE, val=np.ones(nn), desc="TAS rate", units="ft/s**2",
tags=['dymos.state_rate_source:TAS', 'dymos.state_units:kn'])
Copy link
Contributor

Choose a reason for hiding this comment

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

This tag looks wrong now. it's referring to dymos.state_rate_source:TAS, but we're actually calling it Velocity_rate now. Not sure if this will brick something later.

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.

Just a few things i noted. Nothing critical.

@johnjasa johnjasa added this pull request to the merge queue Feb 6, 2024
Merged via the queue into OpenMDAO:main with commit 6b9d0c4 Feb 6, 2024
7 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.

TAS & TAS_rate -> VELOCITY & VELOCITY_RATE
3 participants