JuliaSmoothOptimizers / BundleAdjustmentModels.jl

Julia repository of bundle adjustment problems
Mozilla Public License 2.0
9 stars 5 forks source link

Adding jprod_residual! and jtprod_residual! #64

Open AntoninKns opened 2 years ago

AntoninKns commented 2 years ago

Adding a workaround of jprod_residual! and jtprod_residual! in order for BundleAdjustmentModels to work better with other packages from JSO. The true detailed version of jprod_residual! and jtprod_residual! should be added later in order to avoid allocations and have better performance for these functions.

codecov[bot] commented 2 years ago

Codecov Report

Merging #64 (13fd79f) into main (aec61a2) will increase coverage by 0.44%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main      #64      +/-   ##
==========================================
+ Coverage   92.20%   92.65%   +0.44%     
==========================================
  Files           5        5              
  Lines         295      313      +18     
==========================================
+ Hits          272      290      +18     
  Misses         23       23              
Impacted Files Coverage Δ
src/BundleAdjustmentNLSFunctions.jl 96.80% <100.00%> (+0.53%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update aec61a2...13fd79f. Read the comment docs.

dpo commented 2 years ago

I don't think this is a good idea. This is going to be so much slower than what you have now.

AntoninKns commented 2 years ago

@tmigot asked me to add them in case these functions are called by other JSO packages at least it won't crash. I'll let him correct me, I might not have understood the reasons well.

AntoninKns commented 2 years ago

If you think it's worth the time I can directly implement the jprod_residual! and jtprod_residual! manually based on the jacobian description.

tmigot commented 2 years ago

I asked Antonin this, so that we can use the package. I get that this is a very basic version (and not efficient), but we can improve it later when Antonin is further advanced with his project.