InvDepthFactor3: Fix wrong derivative H3 #1157
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@d-vo can you please do us a favor and add a unit test for this? It should be pretty straightforward since we already have machinery for computing and verifying against numerical derivatives. |
|
Sure. I can add some unit tests for the derivatives, but might take a few days before I get to it. We can leave the PR open until I get to it, or we can merge and I will add them separately - as you prefer. |
|
I would prefer to keep this open since it helps with the git history. Looking forward to the tests! |
|
Started CI, @varunagrawal or I can merge when it passes. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes a wrong assignment in
gtsam_unstable/slam/InvDepthFactor3.hwhere H2 is set to the wrong dimensions and H3 is left unassigned in case of a projection outside of the camera field-of-view.