Feature/bsk 857 polymorphic state data #858
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
juan-g-bonilla
force-pushed
the
feature/bsk-857-polymorphic-StateData
branch
3 times, most recently
from
9b6777c
to
93442a2
Compare
Also improvements to docstrings in StateData
DynParamManager now makes use of some templated code, for which we need to provide some SWIG machinery.
Now, we make use of the StateData propragate function, which can now be overridden to provide custom state integration function.
|
Ola @joaogvcarneiro! I have a test failing for this branch. It's related to the variable integrator scenario, which I believe you wrote? I made some changes to the adaptive integrators. The math should be the same, but the numerics have shifted around. I think the failing test is testing to an accuracy that is within the integrator error, so the updated numerics trigger the comparison failure. That's only my intuition, so I'd appreciate your take on it if you have a minute. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
StateDatawas made polymorphic,StateData::propagateStateis now virtual.This also required improvements to how the integrators work.
ExtendedStateVectorwas given additional methods and updated to handle the polymorphicStateData. Similarly,DynParamManagerhad to be updated (registerStateis now a templated method for different state classes, for example). Unfortunately, this means a SWIG file is now necessary forDynParamManager(templated code needs some extra massaging in SWIG), which in turn means a lot of SWIG interface files must be updated to use this new SWIG file.I'm a good boy scout, so I also tried to clean up code and add extra docstrings in these classes.
Verification
Commit 8976004 (or it's current equivalent) removes support for
operator+andoperator*onStateData, thattest_stateArchitecturedepended on. This test has been updated to use other, equivalent, methods.No new tests added. Essentially all other Basilisk tests use
StateDataand integration, thus these tests passing should prove the refactored system works as expected. For now, only oneStateDataclass appears in Basilisk (with the standard propagation function), and that should be shown to work. NewStateDataclasses should be tested as they are implemented.Documentation
No changes. API remains very similar: by default
DynParamManagerassumes you want to create a standardStateData. The state system is not discussed on the prosaic documentation, thus no changes are needed.Future work
Implement custom
StateDataclasses, such as theMRPStateDataorQuaternionStateDataclasses described in #857