2020-09-02 mathematicalfunctions feedback

[vbaliga]

This is feedback for mathematicalfunctions.R only. I am going to make my way through all the .R scripts over the next few days, and I'll make separate entries for the other function scripts. I elected to start with this one since looking at the underlying math will hopefully help me understand other functions.

General comment for @return section of all functions: A general practice that people do is to have the first few words of this section describe the type of object that is created in the R environment by the function -- if it's a vector, matrix, list, etc. So, for example, line 13 could become "a numeric vector with value 1 if i and j are equal or 0 if they are not".

• Line 5: Sorry if I've misunderstood, but I've heard this called the Kroenecker delta. After googling "Dirac delta" it seems like that refers to something else? This wiki page seems to match what your dirac function does. So should this be renamed kroenecker_delta() or something along those lines?

• Line 35: Can the roxygen comment be a little more specific about what kind of rotation matrix is generated? Cosine?

• Line 70: The example for calc_univec() doesn't look right, as unit_vector() isn't an exported function. Did you mean: output = calc_univec(c(2,6,5) here?

• Line 122: Some of the parameters are missing descriptions

• Lines 408 and 410: I am having trouble understanding the basis for using the fraction 1/8 as feather locations are determined. I'm totally on board with assuming feathers will be evenly distributed across their corresponding bones. But the 1/8 thing eludes me -- why isn't it 1/no_prim or 1/(no_prim - 2)? Also, would it be robust to cases where primary count isn't 10?

[charvey23]

• I will update the 'return' sections as you mentioned
• Line 5: you have taught me a thing. I always used them interchangeably but it turns out the Kronecker delta is the discrete version of the Dirac delta so in this case you are correct! Good catch
• Line 35: To my understanding in 3x3 space all rotation matrices are equivalent. The final output matrix will not be dependent on whether you used sines-cosines or euler angles to construct the matrix. I specified that the construction itself uses a cosine direction matrix. But the output is just a 3x3 transformation matrix and specifically only affects the rotation. For example calc_rot on Line 80 is also a rotation matrix but constructed by projecting one axis system on the other because I don't know the euler angles and it's easier this way. These different constructions differ from a 4x4 space where quarternions come into play (although it can be transformed into that of course). So I added some clarification that these are 3x3 rotation matrices.
• Line 70: Yup!
• Line 122: fixed!
• Line 408: So my logic here was that we could model it as if P1 to P7 are always distributed linearly along the carpometacarpus and then P8 and up would always be located at the tip of the carpometacarpus. I.e. if a bird only had 8 primaries only 1 would be on the tip and if it had 10 then 3 would be located at the tip. I checked that this lines up with a previous study on pigeons that the last three should just be modelled like that but it is an assumption for birds with less than 10. This also would only work for birds that have at least 8 primaries... is that general enough?

[vbaliga]

OK awesome thanks for taking care of these.

Just a minor thing I noticed just now:

• Lines 83 and 84: There's no description of z_vector and x_vector, but we'll be required to have them -- we're getting errors in generating docs. Can you please add these in?

After that, I think we're in great shape for this script. Will jump into another one now!

[vbaliga]

Spoke too soon hahah. There is one more minor thing:

• Line 388: generating documentation for this function is also buggy because there is an @examples roxygen tag but there is no supplied example. Can you either add an example or just delete this line entirely? We can always add in an example later on.

[charvey23]

complete! I will close this issue then. 1 down many to go!