Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Precompute symbolic matrix to hold jacobian #41

Merged
merged 7 commits into from
Jan 20, 2025

Conversation

jumpinthefire
Copy link
Contributor

@jumpinthefire jumpinthefire commented Jan 20, 2025

This PR allows the optimizer to pre-compute a symbolic matrix to hold the Jacobian. This ends up consistently shedding time at each iteration of the optimization loop, across all examples, by invoking SparseColMat::new_from_order_values instead of SparseColMat::try_new_from_triplets.

Since the original insertion order must be respected to make use of the symbolic matrix, we've moved away from a shared list of jacobian values (which may have items inserted into it in any order) to each residual block returning a sublist of values. These sublists are then flattened in the same order as the residual blocks.

Note: compiling the tests in CI/CD was failing because of the plotting library, installing the right native dependencies seems to fix it.

@jumpinthefire jumpinthefire marked this pull request as ready for review January 20, 2025 14:36
src/problem.rs Outdated
for (i, var_key) in residual_block.variable_key_list.iter().enumerate() {
if let Some(variable_global_idx) = variable_name_to_col_idx_dict.get(var_key) {
if let Some(_) = variable_name_to_col_idx_dict.get(var_key) {
Copy link
Owner

Choose a reason for hiding this comment

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

If this is not being used anymore, can you change it to if contains_key else panic (because it should be here) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done ✔️

@powei-lin
Copy link
Owner

There is a printlin in LM. My mistake. Can you help me change that to trace, please?

@powei-lin
Copy link
Owner

I tried and it's like ~10% faster (depends on how large and how empty is the jacobian). Amazing!

@jumpinthefire
Copy link
Contributor Author

There is a printlin in LM. My mistake. Can you help me change that to trace, please?

Done ✔️

@powei-lin powei-lin merged commit 1785aa9 into powei-lin:master Jan 20, 2025
1 check passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants