Enable particle initialization using covariances #259
Merged
Conversation
This file contains 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
nhatao
commented
Oct 10, 2019
include/mcl_3dl/noise_generator.h
Outdated
| { | ||
| public: | ||
| template <typename T> | ||
| explicit DiagonalNoiseGenerator(T& sigma) |
There was a problem hiding this comment.
Currently it is not possible to make this "const T& sigma", as some classes that inherit ParticleBase in our private repository does not have const version of operator[]. We need to fix them first.
[#667] FAILED on melodic
[#667] PASSED on kineticAll tests passed |
Codecov Report
@@ Coverage Diff @@
## master #259 +/- ##
==========================================
+ Coverage 94.24% 94.29% +0.04%
==========================================
Files 25 28 +3
Lines 1563 1629 +66
==========================================
+ Hits 1473 1536 +63
- Misses 90 93 +3
Continue to review full report at Codecov.
|
[#668] PASSED on melodicAll tests passed[#668] FAILED on kineticTest failed[#668] PASSED on kineticAll tests passed |
at-wat
reviewed
Oct 11, 2019
test/src/test_noise_generator.cpp
Outdated
| { | ||
| const double average = std::accumulate(results[i].begin(), results[i].end(), 0.0) / results[i].size(); | ||
| EXPECT_GE(average, -0.1); | ||
| EXPECT_LE(average, 0.1); |
include/mcl_3dl/noise_generator.h
Outdated
| { | ||
| public: | ||
| template <typename T> | ||
| explicit DiagonalNoiseGenerator(T& sigma) |
There was a problem hiding this comment.
I think it's better to handle mean value of the distribution in noise generator class.
It will make it easy to implement other random distribution types like uniform distribution in desired polygons which would be useful to generate initial distribution.
There was a problem hiding this comment.
Mean is used in State6DOF::generateNoise() (i.e. out of noise generator classes), so it is needed to update interfaces to do it. I made new issue (#260).
There was a problem hiding this comment.
API of generateNoise is changed in this PR and it will be changed again to do #260.
IMO, it would better be done in this PR to minimize the number of API change.
At least, it should be done before the next release.
[#669] PASSED on kineticAll tests passed[#669] PASSED on melodicAll tests passed |
|
@at-wat Please take another look. |
[#673] FAILED on melodicTest failed[#673] PASSED on kineticAll tests passed[#673] PASSED on melodicAll tests passed |
|
NoiseGeneratorBase class is added, and mean is included in this class. @at-wat Sorry for the delay. Please take a look when you have a time. |
at-wat
reviewed
Dec 16, 2019
include/mcl_3dl/state_6dof.h
Outdated
| } | ||
| State6DOF noise; | ||
| const auto org_noise = gen(engine); |
There was a problem hiding this comment.
This auto would better be specified explicitly to avoid confusing with State6DOF type.
test/src/test_noise_generator.cpp
Outdated
|
|
||
| namespace mcl_3dl | ||
| { | ||
| void testDiagonalNoiseGeneratorResults(const std::vector<float>& expected_mean, |
There was a problem hiding this comment.
Is it possible to generate diagonal covariance matrix from sigma and wrap testMultivariateNoiseGeneratorResults?
There was a problem hiding this comment.
testDiagonalNoiseGeneratorResults() and testMultivariateNoiseGeneratorResults() are unified
into testNoiseGeneratorResults().
[#678] FAILED on kineticTest failed[#678] FAILED on melodicTest failed |
[#679] PASSED on kineticAll tests passed[#679] PASSED on melodicAll tests passed |
|
@at-wat PTAL. |
[#680] PASSED on kineticAll tests passed[#680] PASSED on melodicAll tests passed |
|
merging |
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.
Closes #258
Closes #202