Conversation
dehann
left a comment
There was a problem hiding this comment.
Mostly looks good but want to talk through idea of each sample gets own CalcFactor with own .cache. This also so that the ensemble solve of points can easily be made multithreaded in future
| solvefor, | ||
| manifold | ||
| manifold, | ||
| ccwl.measurement, |
| """ legacy support for variable values old functor residual functions. | ||
| TBD, this is still being used by DERelative factors. """ | ||
| _legacyParams::X | ||
| _legacyParams::X #TODO rename to varValsHypo for consistent naming? and not not legacy any more |
There was a problem hiding this comment.
One CalcFactor per sample? History idea was CCW per solve and heap CalcFactor per sample. Then we can do multithreading for each sample. Also for more complicated factors that require own cache memory location. Don't want to index into cf.somemem[sampleIdx]
|
|
||
| function _buildHypoCalcFactor(ccwl::CommonConvWrapper, smpid::Integer) | ||
| # build a view to the decision variable memory | ||
| varValsHypo = ccwl.varValsAll[][ccwl.hyporecipe.activehypo] |
There was a problem hiding this comment.
Looks good, I just want talk through the one CalcFactor for each sample solve to be sure we're on the right track
|
A new calcfactor still gets created for each sample, but it currently shares some elements such as measurement. |
Okay, great -- this looks good to me then thanks! |
8276847 to
0888e90
Compare
cef4d52 to
caa397a
Compare
Codecov Report
@@ Coverage Diff @@
## master #1786 +/- ##
==========================================
+ Coverage 75.95% 76.31% +0.35%
==========================================
Files 82 82
Lines 5994 5975 -19
==========================================
+ Hits 4553 4560 +7
+ Misses 1441 1415 -26
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
Will do the same for deconv in a separate PR |
|
NOTE |
This only focused on hex (pose2 and point2) a lot of other things are untested and likely broken.Tests are looking ok.
This also fixes a deconv issue where the variable manifold was used instead of the factor manifold. and close #1785