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

IMU Parameters Documentation #624

Merged
merged 5 commits into from
Dec 2, 2020
Merged

IMU Parameters Documentation #624

merged 5 commits into from
Dec 2, 2020

Conversation

varunagrawal
Copy link
Collaborator

Fixes #213

@varunagrawal varunagrawal added the docs Update to docs or README without code changes label Dec 1, 2020
@varunagrawal varunagrawal self-assigned this Dec 1, 2020
@dellaert
Copy link
Member

dellaert commented Dec 1, 2020

Attach a PDF ?

@varunagrawal
Copy link
Collaborator Author

The PDF is attached. It is the last of the 3 files, or am I misreading your comment?

Copy link
Member

@dellaert dellaert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, but the table about units - which I gather is the only thing you added - does not make sense in this document. I encourage you to read the document (as I did, again, for the review), and then you'll see this table come as a complete surprise. Perhaps this table, which refers to variables in the IMU factor, should go as documentation in the code?

@@ -88,7 +88,7 @@ enable_testing()

option(GTSAM_BUILD_TESTS "Enable/Disable building of tests" ON)
option(GTSAM_BUILD_EXAMPLES_ALWAYS "Build examples with 'make all' (build with 'make examples' if not)" ON)
option(GTSAM_BUILD_TIMING_ALWAYS "Build timing scripts with 'make all' (build with 'make timing' if not" OFF)
option(GTSAM_BUILD_TIMING_ALWAYS "Build timing scripts with 'make all' (build with 'make timing' if not" OFF)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove this from diff

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aye aye

@varunagrawal varunagrawal requested a review from dellaert December 1, 2020 17:15
@varunagrawal varunagrawal added the quick-review Quick and easy PR to review label Dec 1, 2020
Copy link
Member

@dellaert dellaert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yay!

@dellaert
Copy link
Member

dellaert commented Dec 1, 2020

Feel free to merge and delete branch after checks passed :-)

@varunagrawal
Copy link
Collaborator Author

Don't you love how unicode lets us put all the math symbols directly in an easy-to-read manner? :D

@varunagrawal varunagrawal merged commit 873f038 into develop Dec 2, 2020
@varunagrawal varunagrawal deleted the fix/213 branch December 2, 2020 00:04
@@ -88,7 +88,7 @@ enable_testing()

option(GTSAM_BUILD_TESTS "Enable/Disable building of tests" ON)
option(GTSAM_BUILD_EXAMPLES_ALWAYS "Build examples with 'make all' (build with 'make examples' if not)" ON)
option(GTSAM_BUILD_TIMING_ALWAYS "Build timing scripts with 'make all' (build with 'make timing' if not" OFF)
option(GTSAM_BUILD_TIMING_ALWAYS "Build timing scripts with 'make all' (build with 'make timing' if not" OFF)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops... :-)

@mintar
Copy link

mintar commented Nov 26, 2021

I'm pretty sure that this is incorrect:

Vector3 biasAcc_; ///< The units for stddev are σ = m/s² or m √Hz/s²
Vector3 biasGyro_; ///< The units for stddev are σ = rad/s or rad √Hz/s

It should be:

Vector3 biasAcc_; ///< The units for stddev are σ = m √Hz/s²
Vector3 biasGyro_; ///< The units for stddev are σ = rad √Hz/s

See my comment here for an in-depth explanation.

@dellaert
Copy link
Member

@varunagrawal if you agree change it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Update to docs or README without code changes quick-review Quick and easy PR to review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

IMUFactor example noise/bias units
4 participants