Repository: gonum/gonum
Pull request #1818 (2022-07-26 04:39:40)

spatial/r3: Skew is now method
I understand this has not been talked about, but it's clearly bad API design in my eyes. What do y'all think?

Changes:
* deprecate Skew package level function in favor of method to prevent heap allocations


codecov-commenter: # [Codecov](https://codecov.io/gh/gonum/gonum/pull/1818?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gonum) Report
> Merging [#1818](https://codecov.io/gh/gonum/gonum/pull/1818?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gonum) (5c9759c) into [master](https://codecov.io/gh/gonum/gonum/commit/c4e2c8cf3ea5b5daac3aa18419abe4538c7f2b73?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gonum) (c4e2c8c) will **decrease** coverage by `0.01%`.
> The diff coverage is `100.00%`.

```diff
@@            Coverage Diff             @@
##           master    #1818      +/-   ##
==========================================
- Coverage   73.47%   73.46%   -0.02%     
==========================================
  Files         528      528              
  Lines       61965    61974       +9     
==========================================
- Hits        45531    45530       -1     
- Misses      13870    13880      +10     
  Partials     2564     2564              
```


| [Impacted Files](https://codecov.io/gh/gonum/gonum/pull/1818?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gonum) | Coverage Δ | |
|---|---|---|
| [spatial/r3/mat\_safe.go](https://codecov.io/gh/gonum/gonum/pull/1818/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gonum#diff-c3BhdGlhbC9yMy9tYXRfc2FmZS5nbw==) | `55.55% <ø> (-6.18%)` | :arrow_down: |
| [spatial/r3/mat\_unsafe.go](https://codecov.io/gh/gonum/gonum/pull/1818/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gonum#diff-c3BhdGlhbC9yMy9tYXRfdW5zYWZlLmdv) | `65.38% <ø> (-6.42%)` | :arrow_down: |
| [spatial/r3/mat.go](https://codecov.io/gh/gonum/gonum/pull/1818/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gonum#diff-c3BhdGlhbC9yMy9tYXQuZ28=) | `68.68% <100.00%> (+3.13%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/gonum/gonum/pull/1818?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gonum).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gonum)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/gonum/gonum/pull/1818?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gonum). Last update [c4e2c8c...5c9759c](https://codecov.io/gh/gonum/gonum/pull/1818?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gonum). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gonum).

kortschak: @soypat This will need conflicts to be resolved.

Patch:
diff --git a/spatial/r3/mat.go b/spatial/r3/mat.go
index e638a1d8..eaa3f47f 100644
--- a/spatial/r3/mat.go
+++ b/spatial/r3/mat.go
@@ -198,6 +198,23 @@ func (m *Mat) Det() float64 {
 	return a*deta - b*detb + c*detc
 }
 
+// Skew sets the receiver to the 3×3 skew symmetric matrix
+// (right hand system) of v.
+//                  ⎡ 0 -z  y⎤
+//  Skew({x,y,z}) = ⎢ z  0 -x⎥
+//                  ⎣-y  x  0⎦
+func (m *Mat) Skew(v Vec) {
+	m.Set(0, 0, 0)
+	m.Set(0, 1, -v.Z)
+	m.Set(0, 2, v.Y)
+	m.Set(1, 0, v.Z)
+	m.Set(1, 1, 0)
+	m.Set(1, 2, -v.X)
+	m.Set(2, 0, -v.Y)
+	m.Set(2, 1, v.X)
+	m.Set(2, 2, 0)
+}
+
 // Hessian sets the receiver to the Hessian matrix of the scalar field at the point p,
 // approximated using finite differences with the given step sizes.
 // The field is evaluated at points in the area surrounding p by adding
diff --git a/spatial/r3/mat_safe.go b/spatial/r3/mat_safe.go
index 740259c1..1d23d2fe 100644
--- a/spatial/r3/mat_safe.go
+++ b/spatial/r3/mat_safe.go
@@ -60,6 +60,8 @@ func Eye() *Mat {
 //                  ⎡ 0 -z  y⎤
 //  Skew({x,y,z}) = ⎢ z  0 -x⎥
 //                  ⎣-y  x  0⎦
+//
+// DEPRECATED: use Mat.Skew()
 func Skew(v Vec) (M *Mat) {
 	return &Mat{&array{
 		0, -v.Z, v.Y,
diff --git a/spatial/r3/mat_unsafe.go b/spatial/r3/mat_unsafe.go
index c6c6d4f3..59b251f1 100644
--- a/spatial/r3/mat_unsafe.go
+++ b/spatial/r3/mat_unsafe.go
@@ -47,6 +47,8 @@ func Eye() *Mat {
 //                  ⎡ 0 -z  y⎤
 //  Skew({x,y,z}) = ⎢ z  0 -x⎥
 //                  ⎣-y  x  0⎦
+//
+// DEPRECATED: use Mat.Skew()
 func Skew(v Vec) (M *Mat) {
 	return &Mat{&array{
 		{0, -v.Z, v.Y},


Test patch:
diff --git a/spatial/r3/mat_test.go b/spatial/r3/mat_test.go
index 1ac3dd5f..d50d9c4d 100644
--- a/spatial/r3/mat_test.go
+++ b/spatial/r3/mat_test.go
@@ -103,9 +103,10 @@ func TestSkew(t *testing.T) {
 	const tol = 1e-16
 	rnd := rand.New(rand.NewSource(1))
 	for tc := 0; tc < 20; tc++ {
+		sk := NewMat(nil)
 		v1 := randomVec(rnd)
 		v2 := randomVec(rnd)
-		sk := Skew(v1)
+		sk.Skew(v1)
 		want := Cross(v1, v2)
 		got := sk.MulVec(v2)
 		if d := want.Sub(got); d.Dot(d) > tol {