Updated scaling factors for proper geometric representation of uncertainty ellipses #1067

magicbycalvin · 2022-01-25T22:53:37Z

This pull request stems from this comment. After looking into the links in the comment and reviewing some additional material including page 366 of Stochastic Models, Estimation, and Control Vol 1 by Maybeck, I believe that I have properly modified the scaling factor k to produce an uncertainty ellipse corresponding to 3 standard deviations away from the mean for 2D and 3D cases. In addition to updating the values, I included additional comments to clarify why the specific values were chosen.

I would be happy to discuss further if there are any questions.

Thanks!

…erly display 3 standard deviations for both 2D and 3D cases.

dellaert · 2022-01-25T23:26:39Z

python/gtsam/utils/plot.py

-    k=11.82 corresponds to 3 std, 99.74% of all probability
+    For the 3D case:
+    k = 3.527 corresponds to 1 std, 68.26% of all probability
+    k = 14.157 corresponds to 3 std, 99.74% of all probability


Do you have an online reference?

With code from other comment:

pct_to_sigma(sigma_to_pct(1, 1), 3) ** 2 --> 3.5267403802617303 pct_to_sigma(sigma_to_pct(3, 1), 3) ** 2 --> 14.156413609126677

So seems about right if we want this behavior.

dellaert · 2022-01-25T23:27:47Z

python/gtsam/utils/plot.py

@@ -87,7 +88,7 @@ def plot_covariance_ellipse_3d(axes,
        n: Defines the granularity of the ellipse. Higher values indicate finer ellipses.
        alpha: Transparency value for the plotted surface in the range [0, 1].
    """
-    k = 11.82
+    k = np.sqrt(14.157)


above = 14.157 and here it is sqrt(14.157)

Did you take a look at the PR #1063 as I suggested?

I took a look at PR #1063 but am not sure how it is relevant. Perhaps I am missing something? I am new to contributing to open source projects so perhaps there was a file or something I needed to look at. As far as I can tell, the discussion there is regarding removing a negative sign in equations 5.3 and 5.6 in the math.pdf document.

As for the k values I chose, they come from the Chi Squared distribution's p-table for different dimensions. E.g., for a single dimension, k = 9 corresponds to 3 standard deviations, for 2D we have k = 11.820, for 3D we have k = 14.157, and for 4D k = 16.251 (all values taken from the table on page 366 of the Maybeck Vol 1 textbook). Note that I am choosing k to be the variance rather than the standard deviation, which is why you see the square root. I chose to write the comments as I did simply because I wanted to keep them as similar as possible to the previous comments pointing out the k values.

The most useful online reference I could find for choosing these k values can be found here and the Maybeck textbook can be found here.

Please let me know if I can answer any other questions.

Thanks!

Take a look at my note at the top of plot.py. I added some python code for 2D.

In 2D I chose to leave the k=5, because it is consistent with what you said in the comments, and it is referenced in the documentation.

I don't think we should change the semantics of k in the middle of the game.

Given the note and 1 and 2 above, do you agree that code was already good as it was? You would then only have to bring 3D in line.

Just for reference, we can easily calculate that in python with scipy (even though I guess we won't include that as a dependency, we can put the code in the comment to regenerate the values):

def pct_to_sigma(pct, dof): return np.sqrt(scipy.stats.chi2.ppf(pct / 100., df=dof)) def sigma_to_pct(sigma, dof): return scipy.stats.chi2.cdf(sigma**2, df=dof) * 100. for dim in range(0, 4): print("{}D".format(dim), end="") for n_sigma in range(1, 6): if dim == 0: print("\t {} ".format(n_sigma), end="") else: print("\t{:.5f}".format(sigma_to_pct(n_sigma, dim)), end="") print()

0D 1 2 3 4 5 1D 68.26895 95.44997 99.73002 99.99367 99.99994 2D 39.34693 86.46647 98.88910 99.96645 99.99963 3D 19.87480 73.85359 97.07091 99.88660 99.99846

Cool, this code is more general than the one I put in plot.py (which I assume you looked at) and it agrees with it in the 2D case. So I propose:

undo the changes to 2D plotting

we keep k=5 for 2D

k == sigma, not variance

use k = 5 for 3D as well, which is 99.99846 of all probability.

make sure it multiplies sigmas, not variances.

Just for reference, we can easily calculate that in python with scipy (even though I guess we won't include that as a dependency, we can put the code in the comment to regenerate the values):

Actually, maybe we should include this code, and comment on some of its output values, just like @senselessDev did below. I think that it makes sense for users to specify an integer number of standard deviations, and we can document in 2D and 3D how much percentage 1,2,3,4,5 stddevs corresponds to. k can be an input argument defaulting to 5. But we could provide another optional argument, percentage that, when given, calculates k using pct_to_sigma.

0D 1 2 3 4 5
1D 68.26895 95.44997 99.73002 99.99367 99.99994
2D 39.34693 86.46647 98.88910 99.96645 99.99963
3D 19.87480 73.85359 97.07091 99.88660 99.99846

senselessDev · 2022-01-26T20:16:27Z

python/gtsam/utils/plot.py

+    k = 2.296 corresponds to 1 std, 68.26% of all probability
+    k = 11.820 corresponds to 3 std, 99.74% of all probability


pct_to_sigma(sigma_to_pct(1, 1), 2) ** 2 --> 2.295748928898636 pct_to_sigma(sigma_to_pct(3, 1), 2) ** 2 --> 11.829158081900795

dellaert

I propose:

undo the changes to 2D plotting
we keep k=5 for 2D
keep k as sigma, not variance
use k = 5 for 3D as well, which is 99.99846 of all probability.
make sure it multiplies sigmas, not variances.

…variance ellipse.

magicbycalvin · 2022-01-28T19:34:27Z

I have changed all instances of k to be 5 and made sure that k is treated as the standard deviation (sigma) instead of the variance. I also added some comments which reflect and elaborate on said changes. Note that I multiply k by 2 in the patches.Ellipse calls because it uses the diameter as the input instead of the radius.

dellaert

One more iteration I think

dellaert · 2022-01-28T20:00:28Z

python/gtsam/utils/plot.py

-    k=11.82 corresponds to 3 std, 99.74% of all probability
+    For the 3D case:
+    k = 3.527 corresponds to 1 std, 68.26% of all probability
+    k = 14.157 corresponds to 3 std, 99.74% of all probability


I think these (3.5 and 14.1) should still be changed to standard deviation

dellaert · 2022-01-28T20:00:51Z

python/gtsam/utils/plot.py

+    page 366
+    For the 2D case:
+    k = 2.296 corresponds to 1 std, 68.26% of all probability
+    k = 11.820 corresponds to 3 std, 99.74% of all probability


dellaert · 2022-01-28T20:01:24Z

python/gtsam/utils/plot.py

+    page 366
+    For the 2D case:
+    k = 2.296 corresponds to 1 std, 68.26% of all probability
+    k = 11.820 corresponds to 3 std, 99.74% of all probability


dellaert · 2022-01-28T20:02:30Z

python/gtsam/utils/plot.py


        angle = np.arctan2(v[1, 0], v[0, 0])
+        # We multiply k by 2 since k corresponds to the radius but Ellipse uses


Wow. I guess we were looking at the wrong ellipses all that time!

On the bright side, your numbers would have still been correct!

The requested changes have been made. Please let me know if you need anything else.

Thanks!

…in the discussions regarding the values for k.

dellaert

Awesome. Sorry, but indeed I have more comments :-/ If you're tired of this, I can take over :-)

To be consistent with the past, I would actually propose (1) add an argument k, and to use k=2.5 as the default. I would also update the comments to work with integer ks.

I think I had a summary of comments that also included @senselessDev 's code to be included, with an optional percentage argument - did you see that?

dellaert · 2022-01-30T22:13:09Z

python/gtsam/utils/plot.py

-    k=2.296 corresponds to 1 std, 68.26% of all probability
-    k=11.82 corresponds to 3 std, 99.74% of all probability
+    For the 3D case:
+    k = 1.878 corresponds to 1 std, 68.26% of all probability


I would an argument k here and say
k = 1 corresponds to xx%
k = 2 corresponds to xx%
k = 2.5 (default) corresponds to xx%

dellaert · 2022-01-30T22:13:38Z

python/gtsam/utils/plot.py

+    Based on Stochastic Models, Estimation, and Control Vol 1 by Maybeck,
+    page 366
+    For the 2D case:
+    k = 1.515 corresponds to 1 std, 68.26% of all probability


I would an argument k here and say
k = 1 corresponds to xx%
k = 2 corresponds to xx%
k = 2.5 (default) corresponds to xx%

dellaert · 2022-01-30T22:13:49Z

python/gtsam/utils/plot.py

+    Based on Stochastic Models, Estimation, and Control Vol 1 by Maybeck,
+    page 366
+    For the 2D case:
+    k = 1.515 corresponds to 1 std, 68.26% of all probability


dellaert · 2022-01-30T22:16:22Z

Actually - I don't see my earlier comment :-( Maybe it got lost in the git shuffle...

senselessDev · 2022-01-30T22:34:02Z

Actually - I don't see my earlier comment :-( Maybe it got lost in the git shuffle...

This one?

Keep in mind that it would require scipy in the requirements.

dellaert · 2022-01-30T22:52:24Z

Keep in mind that it would require scipy in the requirements.

Ah, that's a deal-breaker :-( Let's hold off on that then.

dellaert · 2022-01-30T22:53:42Z

PS the reason for the 2.5 sigma is (1) because it feels like a more reasonable thing to show, and (b) it would be consistent with the past. Based on what you calculate, I'll just update the text in the docs to reflect the actual percentage that was shown

dellaert · 2022-04-12T17:05:09Z

Merging and taking over in fix/ellipses branch

senselessDev · 2022-04-22T15:56:30Z

@dellaert What is the plan with this? On develop it is still plotting very wrong ellipses (at least for 2D). You want to change it to 2.5 sigma instead of the 5 on the branch and therefore it is not in develop yet?

dellaert · 2022-04-22T16:59:38Z

I took over this issue but I am strapped for time. I want to change everything according to Maybeck, to be uniform across 1D/2D/3D. But it'll have to wait a bit until I have time to fix once and for all.

senselessDev · 2022-04-22T17:05:28Z

You can take a look in #1177, it is a little improved there I think.
But still the sigmas are hard-coded to not require scipy as dependency.

Modified the scaling values for plotting uncertainty ellipses to prop…

d9a00de

…erly display 3 standard deviations for both 2D and 3D cases.

dellaert reviewed Jan 25, 2022

View reviewed changes

senselessDev reviewed Jan 26, 2022

View reviewed changes

senselessDev mentioned this pull request Jan 26, 2022

Added a convenience function for plotting 2D points. #1042

Merged

dellaert requested changes Jan 28, 2022

View reviewed changes

Changed all instances of the Sigma value, k, to 5 for plotting the co…

1b81776

…variance ellipse.

dellaert reviewed Jan 28, 2022

View reviewed changes

Updated comments to reflect standard deviations instead of variances …

e524e28

…in the discussions regarding the values for k.

dellaert reviewed Jan 30, 2022

View reviewed changes

dellaert changed the base branch from develop to fix/ellipses April 12, 2022 16:59

Merge branch 'fix/ellipses' into develop

df7fb47

dellaert merged commit 0ef4d26 into borglab:fix/ellipses Apr 12, 2022

senselessDev added a commit to senselessDev/gtsam that referenced this pull request Apr 22, 2022

update explanation of uncertainty ellipses, try to fix borglab#1067

7fe577b

dellaert self-assigned this Apr 22, 2022

senselessDev mentioned this pull request Apr 22, 2022

Fix uncertainty ellipses in python helpers #1177

Merged

dellaert mentioned this pull request May 11, 2022

All uncertainty ellipses now contain 95% #1193

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Updated scaling factors for proper geometric representation of uncertainty ellipses #1067

Updated scaling factors for proper geometric representation of uncertainty ellipses #1067

magicbycalvin commented Jan 25, 2022

dellaert Jan 25, 2022

senselessDev Jan 26, 2022 •

edited

Loading

dellaert Jan 25, 2022

magicbycalvin Jan 26, 2022

dellaert Jan 26, 2022

senselessDev Jan 26, 2022

dellaert Jan 26, 2022

dellaert Jan 29, 2022

senselessDev Jan 26, 2022 •

edited

Loading

dellaert left a comment

magicbycalvin commented Jan 28, 2022

dellaert left a comment

dellaert Jan 28, 2022

dellaert Jan 28, 2022

dellaert Jan 28, 2022

dellaert Jan 28, 2022

magicbycalvin Jan 28, 2022

dellaert left a comment

dellaert Jan 30, 2022

dellaert Jan 30, 2022

dellaert Jan 30, 2022

dellaert commented Jan 30, 2022 •

edited

Loading

senselessDev commented Jan 30, 2022

dellaert commented Jan 30, 2022

dellaert commented Jan 30, 2022

dellaert commented Apr 12, 2022

senselessDev commented Apr 22, 2022

dellaert commented Apr 22, 2022

senselessDev commented Apr 22, 2022

		k = 2.296 corresponds to 1 std, 68.26% of all probability
		k = 11.820 corresponds to 3 std, 99.74% of all probability


		angle = np.arctan2(v[1, 0], v[0, 0])
		# We multiply k by 2 since k corresponds to the radius but Ellipse uses

Updated scaling factors for proper geometric representation of uncertainty ellipses #1067

Updated scaling factors for proper geometric representation of uncertainty ellipses #1067

Conversation

magicbycalvin commented Jan 25, 2022

Choose a reason for hiding this comment

senselessDev Jan 26, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

senselessDev Jan 26, 2022 • edited Loading

Choose a reason for hiding this comment

dellaert left a comment

Choose a reason for hiding this comment

magicbycalvin commented Jan 28, 2022

dellaert left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dellaert left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dellaert commented Jan 30, 2022 • edited Loading

senselessDev commented Jan 30, 2022

dellaert commented Jan 30, 2022

dellaert commented Jan 30, 2022

dellaert commented Apr 12, 2022

senselessDev commented Apr 22, 2022

dellaert commented Apr 22, 2022

senselessDev commented Apr 22, 2022

senselessDev Jan 26, 2022 •

edited

Loading

senselessDev Jan 26, 2022 •

edited

Loading

dellaert commented Jan 30, 2022 •

edited

Loading