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

Book software bugs report in Chapter 4 #2

Open
zjudmd1015 opened this issue Oct 16, 2017 · 1 comment
Open

Book software bugs report in Chapter 4 #2

zjudmd1015 opened this issue Oct 16, 2017 · 1 comment

Comments

@zjudmd1015
Copy link

About book software of Chapter 4, in the function FKinSpace.m and FKinBody.m, the for loop uses (size(thetalist)) to count how many times it need to pre-multiply or post-multiply. However, (size(thetalist)) returns dimension of thetalist, which is a vector. Besides, for loop takes the first element of the vector there. As a result, times of loop depends on whether you input the thetalist as row vector or column vector, which is not robust enough and can be easy to made a mistake.

For example, The code in the FKinSpace.m is listed below.

for i = size(thetalist):-1:1
    T = MatrixExp6(VecTose3(Slist(:,i) * thetalist(i))) * T;
end

Here we have a thetalist = (th1, th2, th3, th4). If we input it as thetalist = [th1; th2; th3; th4]; , it will be a 41 column vector, and (size(thetalist)) returns [4, 1], so the output result will be right. Otherwise, if we input it as thetalist = [th1, th2, th3, th4];, it's 14 row vector, and (size(thetalist)) returns [1, 4], so the for loop only run one time, which cause a wrong output.
One solution to make it more robust is to replace the size() function to length() function, which only returns a scalar noting length of whatever a row vector or column vector.

for i = length(thetalist):-1:1
    T = MatrixExp6(VecTose3(Slist(:,i) * thetalist(i))) * T;
end

Miaoding Dai _[email protected]

@jarvisschultz
Copy link
Contributor

I agree that this is a problem, but I don't agree with the proposed fix. Nearly every function in the library is sensitive to the shape of the input arrays. Randomly breaking this strict shape-dependence results in an inconsistent API. I think that real issue here is that if you pass the wrong shape for thetalist you get an answer, but not the correct answer. Likely the better approach to addressing this issue is adding more error checking to the library broadly, and coming up with a consistent strategy for handling errors (e.g. throwing exceptions, returning NaN/None/error flags, printing warnings, assertions, etc.).

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

2 participants