Skip to content
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

Parametrize number of semantic labels #55

Open
RozDavid opened this issue Feb 3, 2021 · 4 comments
Open

Parametrize number of semantic labels #55

RozDavid opened this issue Feb 3, 2021 · 4 comments

Comments

@RozDavid
Copy link

RozDavid commented Feb 3, 2021

Hey @ToniRV,

First of all, thank you very much for this work, in simulation it works amazing, but would like to have some question about how to make the transition to real world data.

So I realized, when included a custom CNN ROS node for running the segmentation, that the number of labels in confidence matrices are fixed.

The Struct declaration where you declare this is here.

While I was inspecting how to change this to Dynamic size matrices I realized that the static struct is used several times and in multiple places. For the first try it didn't seem to be too straightforward to fix this problem, but if you could help with a few guiding ideas where to replace I would be able to provide a pull request with the fix.

My first guesses would be:

Could you help what I would be missing and what could go wrong after change?

Thanks a lot,
David

@codieboomboom
Copy link

@RozDavid I realized the same thing today when working on a different segmentation model trained on ADE20K. Seems like the value is defined in this header file for kimera_semantics module.

Something i observed for my side is when changing that constant to my model's number of label, running the kimera semantics will fail this assertion code in semantic_integrator_base.cpp. Not sure if you know why it happens, I just disabled it for now.

CHECK_NEAR(
      semantic_log_likelihood_.sum(),
      kTotalNumberOfLabels * log_match_probability_ +
          std::pow(kTotalNumberOfLabels, 2) * log_non_match_probability_ -
          kTotalNumberOfLabels * log_non_match_probability_,
      1000 * vxb::kFloatEpsilon);

@RozDavid
Copy link
Author

RozDavid commented Feb 4, 2021

Hey @AnhTuDo1998,

Yeah, I started to work on implementing the dynamic number of labels yesterday, and met with the same problem when we set the total number over 25ish. It is only a rounding error I believe, so we just need to adapt the treshold with the number of labels there, or just manually set it a megnitude larger. For sanity check I think it will still stand.

I believe the bigger challenge here is the initialization of SemanticVoxel priors when assigning new Voxblox blocks with a constant matrix, that's size and value depends on the parameter. With non-fixed sized Eigen matrices we cannot use the original Structs that was defined here.

@ToniRV
Copy link
Collaborator

ToniRV commented Feb 4, 2021

Hi! That sounds like a great idea! I'll get back to you later with suggestions 👍
The idea of fixed Eigen types was just to make it faster, but dynamic matrices should also work.

@ToniRV
Copy link
Collaborator

ToniRV commented Feb 8, 2021

I added my comments to #56, that is definitely the way to go!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants