MET-32: FMB #24
MET-32: FMB #24
Conversation
MET-32 Implement `FMB`
Definition of done
FMB chekclist
FBMs checklist
|
horizon-blue
left a comment
horizon-blue
left a comment
There was a problem hiding this comment.
Thanks for the great work!
As we discussed in the previous meeting, it'd be great to have some documentations for the classes in fmb.cuh. Other than that, this PR looks great to me!
| } | ||
| }; | ||
|
|
||
| class ConstIterator { |
There was a problem hiding this comment.
This is ver thoughtful. I like that we're using const whenever possible :).
One pattern I've seen people using is to make the pointer type generic so you don't have to define the const/non-const version twice... though it might not worth the headache at this point haha.
genmetaballs/src/cuda/core/fmb.cuh
Outdated
| #include <cuda/std/span> | ||
| #include <cuda/std/tuple> | ||
| #include <stdexcept> | ||
| #include <tuple> |
There was a problem hiding this comment.
Is this non-cuda tuple header still being used? ;)
| FMB() : pose_{}, extent_{1.0f, 1.0f, 1.0f} {}; | ||
|
|
||
| FMB(const Pose& pose, float x_extent, float y_extent, float z_extent) noexcept(false) |
There was a problem hiding this comment.
👀 are we not going to construct FMB on the device?
There was a problem hiding this comment.
As of the current pseudocode, we are initializing it in host, but we could totally make this a CUDA CALLABLE too.
There was a problem hiding this comment.
yeah I think host only should be fine for now.
| } | ||
|
|
||
| template <> | ||
| __host__ FMBScene<MemoryLocation::HOST>::FMBScene(size_t size) |
There was a problem hiding this comment.
Nice set of helpers! We should probably do this with all our structs tbh, would be much cleaner
|
One question regarding the conversion form Covariance --> Quat + Extents (via eigendecomp). how are we thinking about this again? @mugamma Like is this something that only needs to happen when initializing the FMBs? From the last time we met, I believe that was all that's needed? |
It's mainly for higher quality training for now, and yes: if you want to initialize FMBs differently using precision or covariance matrices you will need to do an eigendecomposition. |
…ion, whereas `Pose extr;` create a new variable)
horizon-blue
force-pushed
the
mugamma/MET-32-fmb
branch
from
94165c6 to
0037377
Compare
Please and thank you.