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

Bug/thruster state effector isp factor #279

Merged
merged 4 commits into from
Jul 25, 2024

Conversation

andrewmorell
Copy link
Collaborator

  • Tickets addressed: bsk-735
  • Review: By commit
  • Merge strategy: Merge (no squash)

Description

A hard coded value is removed from the thrusterStateEffector that was erroneously forcing mass depletion always on. Then mass depletion logic is updated to check ThrustFactor to see if a thruster is firing rather than IspFactor which is unused by the thrusterStateEffector. Additionally, the mass depletion is changed to now scale its expenditure according to the scaled value of thrust following ramp up/down.

Verification

The FuelTank mass depletion unit test already produces plots of the fuel expenditure for a thruster dynamic and state effector, and the state effector's plots for mass and massDot vs. time now accurately reflect the firing pattern. While mass depletion was being plotted, no assertions were made as to accuracy of the values, so two assertions have been added to the fuel tank's mass depletion unit test checking that massDot is ramped up when the thruster is firing and ramped down when the thruster is commanded off.

Additionally, the thrusterStateEffector unit test is updated to scale the thrust by its ramp up/down factor when computing truth values for massDot comparison when toggled on for mass depletion force contributions.

Documentation

No documentation is invalidated by these changes, just fixing code to match what the documentation outlines.

@andrewmorell andrewmorell added the bug Something isn't working label Jul 8, 2024
@andrewmorell andrewmorell self-assigned this Jul 8, 2024
@andrewmorell andrewmorell force-pushed the bug/thrusterStateEffector-IspFactor branch from 76b8ea3 to 252d223 Compare July 8, 2024 16:34
Copy link
Collaborator

@codyallard28 codyallard28 left a comment

Choose a reason for hiding this comment

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

I just have one question about a comment but otherwise, looks good to me.

This is significant in ensuring that mass is depleted at an appropraite rate. Because of how
the thrusterStateEffector ramps down, it asymptotically approaches zero, meaning that when
the mass depletion logic checks for non-zero thrust, it will always believe the thruster to
be on during ramp down even at extremely small values. Therefore the thrust magnitude must
be scaled during mass dot calculations to ensure virtually no mass is depleted since it will
still try to compute this.
@andrewmorell andrewmorell force-pushed the bug/thrusterStateEffector-IspFactor branch from 252d223 to 0d78674 Compare July 25, 2024 14:57
@patkenneally patkenneally merged commit 8a258b0 into develop Jul 25, 2024
3 checks passed
@patkenneally patkenneally deleted the bug/thrusterStateEffector-IspFactor branch July 25, 2024 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants