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

Possible bug in stochman.curves.CubicSpline #29

Open
fadel opened this issue Oct 18, 2023 · 0 comments
Open

Possible bug in stochman.curves.CubicSpline #29

fadel opened this issue Oct 18, 2023 · 0 comments

Comments

@fadel
Copy link

fadel commented Oct 18, 2023

Hi,

I was going through the source code of stochman with a colleague and we found what could likely be a bug in stochman.curves.CubicSpline. In essence, the code for computing the coefficients used in the second-order derivative seems to have the wrong position for the coefficients.

I would suggest the following patch:

diff --git a/stochman/curves.py b/stochman/curves.py
index 97c22bd..a40f7ca 100644
--- a/stochman/curves.py
+++ b/stochman/curves.py
@@ -356,7 +356,7 @@ class CubicSpline(BasicCurve):
             second = torch.zeros(num_edges - 1, 4 * num_edges, dtype=self.begin.dtype)
             for i in range(num_edges - 1):
                 si = 4 * i  # start index
-                fill = torch.tensor([0.0, 0.0, 6.0 * t[i], 2.0], dtype=self.begin.dtype)
+                fill = torch.tensor([0.0, 0.0, 2.0, 6.0 * t[i]], dtype=self.begin.dtype)
                 second[i, si : (si + 4)] = fill
                 second[i, (si + 4) : (si + 8)] = -fill

In our (limited) tests, using this version leads to more accurate logmaps.

What do you think? Was that intentional?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant