Feature/coma model #490
Draft
Feature/coma model #490
Conversation
This file contains hidden or 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
…erical harmonics cache update
|
@Markus-Reichel I see you're addressing the errors in the compilation, which seem to come from manual uses of the createAtmosphereModel, which now requires a SystemOfBodies as input. As an alternative to adding these as inputs, you can also add an empty SystemOfBodies as input by default. |
DominicDirkx
reviewed
Oct 9, 2025
| throw std::runtime_error( "Error when making coma wind model for body " + body + ", atmosphere model must be a ComaModel" ); | ||
| } | ||
| std::function< Eigen::Vector6d( ) > sunStateFunction = | ||
| std::bind( &simulation_setup::Body::getState, bodies.at( "Sun" ) ); |
There was a problem hiding this comment.
Add a check whether the body "Sun" and body exist in the bodies before doing this, to prevent difficult to track down segmentation faults.
cmake/tudat/TudatFindBoost.cmake
Outdated
| add_library(Boost::${_TUDAT_BOOST_COMPONENT} STATIC IMPORTED) | ||
| else() | ||
| add_library(Boost::${_TUDAT_BOOST_COMPONENT} UNKNOWN IMPORTED ../tests/src/simulation/unitTestPolyhedron.cpp) | ||
| add_library(Boost::${_TUDAT_BOOST_COMPONENT} UNKNOWN IMPORTED ../tests/src/simulation/unitTestPolyhedron.cpp |
|
|
||
|
|
||
| add_subdirectory(satellite_propagation) | ||
| add_subdirectory(applications) |
There was a problem hiding this comment.
Could you remove this from the PR (maybe add this file to teh gitignore)?
There was a problem hiding this comment.
this is so I can track my applications which I use to do the DSMC comparison or the performance analysis.
I will add it to git ignore at the very end once everything else works as expected
|
The CI on Windows is failing due to use of C++17 feature (maybe in other places as well?): and due to the use of M_PI, which is not platform-independent. Use tudat::mathematical_constants::PI instead |
…ptr holder in pybind11 binding │ │ │ │ The ComaModelFileProcessor class was experiencing memory corruption where the │ │ filePaths_ vector would be populated correctly during construction but become │ │ empty when accessed later in createPolyCoefDataset(). This was followed by a │ │ double-free error during object destruction. │ │ │ │ Root cause: │ │ - Factory functions (comaModelFileProcessorFromPolyFiles/FromSHFiles) return │ │ std::shared_ptr<ComaModelFileProcessor> │ │ - The pybind11 binding did not specify a holder type, defaulting to std::unique_ptr │ │ - When pybind11 converted between holder types, it triggered a copy/move of the │ │ ComaModelFileProcessor object │ │ - The class contains a ComaStokesDataset member with large Eigen matrices that │ │ gets default-constructed even for polynomial processors │ │ - During the copy/move operation, the Eigen matrix copy corrupted the filePaths_ │ │ vector in memory, causing it to become empty │ │ - Later, the corrupted object caused a double-free during destruction │ │ │ │ Solution: │ │ Add std::shared_ptr as the explicit holder type in the pybind11 class binding, │ │ matching the pattern used by other classes in the codebase. This prevents │ │ pybind11 from copying/moving the object and eliminates the memory corruption.
…Model Changes: - Fix radius calculation to use currentBodyCenteredAirspeedBasedBodyFixedState_ instead of currentBodyCenteredState_ in FlightConditions::computeRadius() - Add radius validation in ComaModel::getDensity() to throw error when radius is smaller than reference radius - Increase scalarFlightConditions_ size from 13 to 14 in FlightConditions constructor
Add new dependent variable to save vehicle part rotation matrices at each
propagation time step. This allows tracking the orientation of individual
vehicle parts (e.g., solar panels, antennas) relative to the body-fixed
frame during simulation.
Implementation follows the pattern of existing rotation matrix dependent
variables (inertial_to_body_fixed_rotation_matrix_variable). The rotation
data is retrieved from VehicleSystems::currentVehiclePartRotationToBodyFixedFrame_
which stores the current rotation quaternion for each vehicle part at every
time step.
Changes:
- Add vehicle_part_rotation_matrix_dependent_variable enum (value 79)
- Add vehiclePartRotationMatrixVariable() interface function
- Implement case handler in getVectorDependentVariableFunction that:
* Retrieves rotation via VehicleSystems::getPartRotationToBaseFrame()
* Converts quaternion to 9-element vector representation
* Validates body has VehicleSystems before accessing
- Add size (9), shape (3x3), and matrix type metadata
- Add Python bindings: vehicle_part_rotation_matrix(body, part_name)
Usage:
dependent_variable.vehicle_part_rotation_matrix(Vehicle, SolarPanel)
The part_name parameter uses the secondaryBody_ field of
SingleDependentVariableSaveSettings. Empty string retrieves main body rotation.
Files modified:
- include/tudat/simulation/propagation_setup/propagationOutputSettings.h
- include/tudat/simulation/propagation_setup/propagationOutput.h
- src/tudat/simulation/propagation_setup/propagationOutput.cpp
- src/tudat/simulation/propagation_setup/propagationOutputSettings.cpp
- src/tudatpy/dynamics/propagation_setup/dependent_variable/expose_dependent_variable.cpp
Fix missing case in createEnvironmentUpdater.cpp that caused 'parameter 79 not found' error. Adds body_rotational_state_update to trigger VehicleSystems part orientation updates.
Implement comprehensive performance optimizations and code quality improvements to the ComaModel atmospheric density computation pipeline.
Markus-Reichel
force-pushed
the
feature/coma_model
branch
from
19b73d7 to
aff4b61
Compare
Moved co-rotation behavior from AtmosphereModel to WindModel class hierarchy
for better architectural design. Key changes:
- Removed includeAtmosphericRotation parameter and methods from AtmosphereModel
and all AtmosphereSettings classes
- Added includeCorotation parameter to WindModel base class and all derived
wind model classes (ConstantWindModel, CustomWindModel, ComaWindModel)
- Created new EmptyWindModel class for specifying co-rotation behavior without
physical wind
- Updated all WindModelSettings classes to support includeCorotation parameter
- Modified AtmosphereSettings to use EmptyWindModelSettings by default with
co-rotation enabled for backward compatibility
This refactoring provides a cleaner separation of concerns, as co-rotation is
fundamentally a component of the wind velocity field rather than a general
atmosphere property.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
The coma model is dependent on the state of the Comet and the Sun. Thus, the Sun needs to be part of the system of bodies and needs to be updated each step.