spatial/r3: Add method to Rotation for stringing multiple rotations

[soypat]

Add a way to combine rotations to avoid this: r := Rotation(quat.Mul(quat.Number(q), quat.Number(r)))

// Then combines rotations r and q such that 
// q follows r.
func (r Rotation) Then(q Rotation) Rotation {
	return Rotation(quat.Mul(quat.Number(q), quat.Number(r)))
}

[sbinet]

in my view, flow API aren't so idio(go)matic.
other may want to chime in but, what about:

func RotationFrom(rs ...Rotation) Rotation {
    if len(rs) == 0 { panic("...") }
    r := rs[0]
    for _, q := range rs[1:] {
        r = Rotation(quat.Mul(quat.Number(q), quat.Number(r)))
    }
    return r
}

and/or, if the number of composition of rotations is usualy just 2:

func Compose(r1, r2 Rotation) Rotation {
    return Rotation(quat.Mul(quat.Number(r2), quat.Numer(r1)))
}

(the latter being probably more amenable to inlining)

but perhaps I just have a grip with the name of the method (and I'd settle for func (r Rotation) Cross(q Rotation) Rotation {...})

apologies for the stream of consciousness.

[kortschak]

I would prefer not to add layers of abstraction here.

[soypat]

I agree the function is sufficiently short to be written by users. My main gripe was that I spent quite a while researching quaternion rotation composition. It wasn't until I came across a stackoverflow post with the conjugate equation for two quaternion rotations. It'd be nice if users have a way of combining two rotations that the library guarantees works since it saves the less-traveled programmers of us some research and worries about correctness.

Edit: I agree flow API is not idiomatic in many cases. The notable case being the Vec type whose now deprecated methods lend themselves for award-winning messy code. However, I think in this case it may be idiomatic since it's a single method flow API. Should be easy to read:

rot := r1.Then(r2).Then(r3) // very clear order of rotations. Usually no more than 3 rotations to compose?

[sbinet]

alternatively: what about an example?

[soypat]

I mean sure, an example would be nice, but if I'm being honest my first impression when using the Rotation type was

"huh, there's no way to compose rotations? Probably has no obvious correct way with quaternions or there is not a closed form solution if it's not part of the API"

Personally, I'm perplexed as to why adding the closed and correct form of combining rotations would not be a welcome change to gonum. It seems like something so basic. I have never had the "one true axis and angle" needed to perform a rotation and have often depended on euler angles and rotation tensors in my work. Why not provide this basic building block for these rotations? It would even make the euler angle example easier to follow as the rotation could be constructed using this composition. See https://github.com/gonum/gonum/blob/master/spatial/r3/euler_example_test.go#L19.

Sorry for the harsh experience report of gonum. I am a bit frustrated a simple and seemingly harmless change which would give coders like me a bump in quality of life for abstraction pocket change is looked down upon.

[sbinet]

FTR, I was hinting at a dedicated example, say:

func ExampleRotation_compose() { ... }

(so it's displayed together with the Rotation type.)

but, FTR, I am fine with adding a dedicated constructor.

[kortschak]

I think an example is an excellent thing to add for this. It's worth pointing out that there is a way to compose rotations, Mul. It may be that there is a need for a higher level abstraction on quat, but it doesn't need to be here. It's also worth noting that there is a ink to truly outstanding teaching material in the quat docs that should make the things that need to be implemented clear.