-
Notifications
You must be signed in to change notification settings - Fork 62
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
Feature/gauss markov refactor #862
Conversation
6ad08e6
to
537d86d
Compare
537d86d
to
eee4f7a
Compare
eee4f7a
to
551cb71
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Close, need to check other code. We have more modules that use GaussMarkov. I'm surprised the associated unit tests still pass. I guess they were not checking the noise properly?
src/architecture/utilitiesSelfCheck/_Documentation/gaussMarkov/secModelDescription.tex
Show resolved
Hide resolved
src/architecture/utilitiesSelfCheck/_Documentation/gaussMarkov/secModelFunctions.tex
Show resolved
Hide resolved
269307b
to
fdf76b6
Compare
fdf76b6
to
af24174
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work, all tests and documentation worked as expected on my system as well. This is ready to be pushed.
Description
The GaussMarkov class is rewritten in this PR to provide readable syntax, random walk bounds enforcement, and a resolution to the previously required 1.5x multiplier on process noise standard deviation. All downstream instantiations of the GaussMarkov class in sensors are updated accordingly. More precisely, the 1.5x multiplier is removed and bounds are actually implemented. Finally, this PR introduces a new scenario that serves as both an example of the functionality of GaussMarkov for noise modeling as well as a proof of such functionality.
Verification
Relevant automated unit tests were edited to not use a 1.5x multiplier and to have relaxed tolerances such that bounded-random walk functionality is actually tested. The new scenario introduced also has its own automated unit test that generates an image showcasing GaussMarkov behavior.
Documentation
The documentation prescribing the 1.5x multiplier for the GaussMarkov noise std is invalidated by this pr. The reviewer should ensure the assignee edited any documentation of the 1.5x multiplier such that this prescription is removed as it is deprecated in this pr.
Future work
Resolve any concerns or raised issues of developers using the newly updated Gauss Markov class.