search

finsberg / ldrb

A software for assigning myocardial fiber orientations based on the Laplace Dirichlet Ruled-Based algorithm
http://finsberg.github.io/ldrb
GNU Lesser General Public License v3.0
22 stars 8 forks source link

calculus: bislerp fixes #72

Closed jamtrott closed 1 year ago

jamtrott commented 1 year ago

Henrik,

I found a couple of suspicious lines in the bislerp() function in calculus.py.

Can you please take a look at the proposed changes to verify if they are correct?

Regards, James

finsberg commented 1 year ago

Henrik,

I found a couple of suspicious lines in the bislerp() function in calculus.py.

Can you please take a look at the proposed changes to verify if they are correct?

Regards, James

Thanks @jamtrott ! This is for sure a bug! I will just fix the test first before merging this.

finsberg commented 1 year ago

@jamtrott I made some more changes and implemented some of the heuristics that Iver implemented in his version. Could you take a look and see if this make sense?

finsberg commented 1 year ago

@jamtrott This bug has been a real pain, but now I think I have finally figured it out and also removed all the heuristics. Basically, I had to swap the order of the product in the bislerp algorithm. The lifex project is doing the same trick here: https://gitlab.com/lifex/lifex/-/blob/main/source/numerics/quaternions.cpp#L208

Now the algorithm runs fine and produces the expected results without any heuristics. Just need to fix the test.

jamtrott commented 1 year ago

Great work, Henrik.

If I understand correctly, you are saying that electronic supplement (https://link.springer.com/article/10.1007/s10439-012-0593-5) for the paper by Bayer et al. (2012) contains a mistake in "Function 4: bislerp"?

So, in line 3, it says:

"Find q_M ∈{±q_A,±i·q_A,±j·q_A,±k·q_A} that maximizes ∥q_M·q_B∥"

But the order of the quaternion products should be reversed to the following:

"Find q_M ∈{±q_A,±q_A·i,±q_A·j,±q_A·k} that maximizes ∥q_M·q_B∥"

Is that right?

finsberg commented 1 year ago

Great work, Henrik.

If I understand correctly, you are saying that electronic supplement (https://link.springer.com/article/10.1007/s10439-012-0593-5) for the paper by Bayer et al. (2012) contains a mistake in "Function 4: bislerp"?

So, in line 3, it says:

"Find q_M ∈{±q_A,±i·q_A,±j·q_A,±k·q_A} that maximizes ∥q_M·q_B∥"

But the order of the quaternion products should be reversed to the following:

"Find q_M ∈{±q_A,±q_A·i,±q_A·j,±q_A·k} that maximizes ∥q_M·q_B∥"

Is that right?

Correct!

finsberg commented 1 year ago

The tests for the angles now also makes much more sense: 4924f04