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

ref: Move the full jacobian and bound track parameters to the parameter_transporter and merge it with the parameter_resetter #880

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

niermann999
Copy link
Contributor

@niermann999 niermann999 commented Nov 7, 2024

Move the data to the actor that updates them, to pool the update functionality with the corresponding data in a single place.

It is currently possible to call an actor like the pointwise_material_interactor in the actor chain while forgetting to register the parameter_transporter in the same actor chain, even though the material interactor depends on the correct update of the bound track parameters. This PR makes sure, that dependent actors like the material interactor will no longer compile without the parameter_transporter by making their call operator take the parameter transporter state as an argument, which now contains the bound track parameters. Furthermore, this is abstracted in a new type, the paramerter_updater, that additionally makes sure that the parameter_resetter is called correctly, as well. Any actors that depend on the updated bound parameters and covariance can be given to this new type as template parameter list and will automatically be inserted correctly in the actor chain.

In the future, it should be possible to add a second state to the parameter transporter, which does not own the bound track parameters but takes a pointer from the CKF to the current bound track parameters. This should avoid copying the bound track parameters between the CKF and the propagation. At the same time, the stepper becomes easier to reuse in a simulation context with this PR, since it is not mandatory anymore to hold the bound track parameters and full jacobian in memory.

@niermann999

This comment was marked as outdated.

@niermann999 niermann999 force-pushed the ref-move-cov-transp-data branch from c1e16f3 to aa850d6 Compare November 7, 2024 19:17
@niermann999 niermann999 added refactor refactoring the current codes priority: low Low priority blocked This item is blocked by another item labels Nov 11, 2024
@niermann999 niermann999 force-pushed the ref-move-cov-transp-data branch from aa850d6 to 98994d4 Compare November 11, 2024 15:02
@niermann999 niermann999 marked this pull request as ready for review November 11, 2024 15:02
@niermann999 niermann999 force-pushed the ref-move-cov-transp-data branch 3 times, most recently from bb3139f to c044806 Compare November 15, 2024 16:45
@niermann999 niermann999 removed the blocked This item is blocked by another item label Nov 15, 2024
…orter and merge the actors that modify the bound track parameters
@niermann999 niermann999 force-pushed the ref-move-cov-transp-data branch from c044806 to 25e7108 Compare November 18, 2024 15:38

/// Full jacobian
bound_matrix_type m_full_jacobian =
matrix_operator().template identity<e_bound_size, e_bound_size>();
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we don't have to initialize this as the identity matrix if the compiler says OK

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just moved this line over from the stepper, will update

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: low Low priority refactor refactoring the current codes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants