-
Notifications
You must be signed in to change notification settings - Fork 5
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
Adding jprod_residual! and jtprod_residual! #64
base: main
Are you sure you want to change the base?
Conversation
Codecov Report
@@ 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
Continue to review full report at Codecov.
|
I don't think this is a good idea. This is going to be so much slower than what you have now. |
@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. |
If you think it's worth the time I can directly implement the |
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's at least have a caching system for the Jacobian: only evaluate it if x
changes. You should be able to compute the shasum
of x
and compare that instead of checking each component, and only be wrong with probability ε.
Adding a workaround of
jprod_residual!
andjtprod_residual!
in order for BundleAdjustmentModels to work better with other packages from JSO. The true detailed version ofjprod_residual!
andjtprod_residual!
should be added later in order to avoid allocations and have better performance for these functions.