Feature/dino gem agg descriptors #37
Conversation
|
Manually checked submap descriptor param options, all still working |
roman/map/fastsam_wrapper.py
Outdated
| # run fastsam | ||
| masks = self._process_img(img, ignore_mask=ignore_mask, keep_mask=keep_mask) | ||
|
|
||
There was a problem hiding this comment.
I don't feel really picky about whitespace, but we might as well delete these tabs while making other changes haha 🤷♂️
| self.observations.append(Observation(t, pose, mask, mask_downsampled, ptcld)) | ||
|
|
||
| return self.observations | ||
| return self.observations, frame_descriptor |
There was a problem hiding this comment.
This will break things downstream in roman_ros, so before we merge this PR, we should make updates there too.
roman/utils.py
Outdated
|
|
||
| import matplotlib.pyplot as plt | ||
| import numpy as np | ||
| import torch |
There was a problem hiding this comment.
I don't think this gets used?
There was a problem hiding this comment.
My bad, I had some L_p norm code here but i moved it directly into fastsam_wrapper
mbpeterson70
left a comment
mbpeterson70
left a comment
There was a problem hiding this comment.
Just commented a few minor things to change before this gets merged in! We should also make a plan for integrating this into the ros code
mbpeterson70
left a comment
mbpeterson70
left a comment
There was a problem hiding this comment.
@liqyn thanks for making those changes! This looks good to merge in now, but let's wait until we can merge relevant changes into roman ros before merging here. I think even just handling the fact that the fastsam wrapper outputs two things instead of one would be sufficient so that the two main branches remain compatible while we work in integrating the full image-based descriptor functionality into the ros version.
|
This should be good to merge in together with mit-acl/roman_ros#23 (review)! |
2 participants
Add submap VPR using DINO for CLIPPER pre-filtering