Re: Remove Transpose Methods?

[Patricio Whittingslow]

Btracey had previously made an issue questioning whether the T() method should be defined on concrete types and in the Matrix interface. I'm not here to talk about concrete types, but specifically about the mat.Matrix interface which is a source of concern for me.

A matrix is a very basic data type. It could arguably be implemented with the Dims(), At(), methods. However, the mat.Matrix interface includes a third method, the T() method. This would be all fine and dandy except for the fact that it includes the mat.Matrix type within the function signature.

This to me is problematic because to implement the Matrix interface one needs to import the mat package, all for a T() method that really is not needed for most matrix operations- if a API needs the T(), they could just mat.Transpose{m} the input matrix to achieve the same effect.

Other notable users agree it would be great to be able to implement the Matrix interface without importing mat.

Removing the T() would be a huge breaking change. Here are some impact mitigation strategies
* If Pat were king of the world: Remove T() method from mat.Matrix. Add new interface called mat.MatrixT that implements new mat.Matrix with the T method. Benefit of this change is that it'd be trivial to instruct users how to fix their code gofmt -w -r 'mat.Matrix -> mat.MatrixT' ./...

* Plan for a breaking change in v1.0.0 with changes similar to above.
* Do nothing. This is safest choice. mat package will continue to propagate itself into hidden corners of the go ecosystem like a GPL license. The year is 2029 and `mat` package runs rampant...

[Dan Kortschak]

I'd like to understand the cost that is being mitigated by this
proposal. The changes that are required would impose a significant
burden on maintainers* in both code mutation and in review because of
the broad impact of the change, and would have an impact on performance
in cases where the transpose can be elided and the matrix type is aware
of that since for code to be generically usable the matrix consumer
would have to assume that all matrix types need to be actively
transposed if any do for an algorithm. I'd argue (more subjectively)
that it will make mat code harder to read — this is a position that I
held previously and which hasn't changed over the 3 years since that
issue was discussed.

* This is at a point where there are fewer of us and those of us who
are here are less able to spend time on the project.

Also, it's worth pointing out that there is a fourth option that I
think (though haven't experimented to confirm) addresses the concerns
here without the legibility or potential performance costs (though
still with a maintainer burden and some, though less, user-breaking
costs). This is to implement the mat API on Go type-parameterised
methods so that mat.Matrix is something like

type Matrix[T any] interface {
At(i, j int) float64
Dims() (r, c int)
T() T
}

(possibly with a further type differentiation so that we have element
type specification, though this requires some additions to the language
to make it work in the general case see https://go.dev/issue/45049)

type Float interface {
~float64 | ~float32
}

type Ater[E Float] interface {
At(i, j int) E
}

type Matrix[T Ater[E], E Float] interface {
At(i, j int) E
Dims() (r, c int)
T() T
}

I have not thought fully through the implications of this, and I feel
that there are likely to be some subtleties in how instantiated types
interact with each other in ways that are difficult (e.g. how does
T1.Mul(T2, T3) work). So some initial experimentation would be
required, but given that we can't make this generally available before
go1.19 is release and we drop go1.17, we have some time.

This all obviously still has a significant maintainer cost in the
changes required and the review of those changes, but brings the
advantages of a reduced API surface (T<Type> methods go away among
other things) and users no longer need to import mat (though I have to
say it's unclear to me why they would implement mat.Matrix and not
import that package in some way).

Dan

[Dan Kortschak]

On Sun, 2022-05-08 at 22:16 +0000, 'Dan Kortschak' via gonum-dev wrote:
> I have not thought fully through the implications of this, and I feel
> that there are likely to be some subtleties in how instantiated types
> interact with each other in ways that are difficult (e.g. how does
> T1.Mul(T2, T3) work). So some initial experimentation would be
> required, but given that we can't make this generally available
> before
> go1.19 is release and we drop go1.17, we have some time.

After some thought, this won't work, because it is not possible to have
type-parameterised methods. Oh well.

[Patricio Whittingslow]

Thanks for the detailed response! I'm currently trying to figure out if there's something I'm missing regarding the performance issue

I understand your point as being:

 > Matrix types would be unaware of ability to transpose and so consumers would perform active transposes on matrixes incurring performance hits. [To get around this (by using type assertions I'd imagine)] mat code would be harder to read.

Please correct me if I misunderstood your points in your first paragraph.

My question is why can't we take Brendan's idea and have the ugly code live in the package T function?

func T(m Matrix) Matrix {

 if u, ok := m.(Untransposer); ok {

 return u.Untranspose()

 }

if u, ok := m.(Transposer); ok {

 return u.T()

 }

return Transpose{m}

}
Would this not solve the performance hits and impact code legibility in a minimal fashion?

t := m.T()  becomes   t := mat.T(m)

As for your second point regarding Matrix[T any] type, it would seem that does solve this issue. That said I don't understand how the T is guaranteed to fulfill the Matrix interface. I think you need #28254 for that, which sadly has been marked as Go2, as so many interesting proposals.

As for the third point on generics, woah, neat proposal!

[Patricio Whittingslow]

> though I have to say it's unclear to me why they would implement mat.Matrix and not
import that package in some way

I err on the side of believing that's how interfaces work- You provide types to users that implement common interfaces so that the user can use your type for what they see fit. The package that provides the type might have no need for the mat package. I can testify in two cases where I developed my own Matrix types and wanted to implement the mat.Matrix interface for good measure but had no use of the mat package anywhere in sight. The use of the mat package in these cases was left up to the end-user, as is common with interface implementation.

Having the mat.Matrix type in the interface makes for noise in the interface implementation. Upon implementing this simple interface you have unknowingly brought in version-control into the interface. Your Matrix implementation when transposed returns a gonum v0.11.0 mat.Matrix type. This can lead up to to inconsistent compatibility between packages due to the Go version in the imported modules. Usually the Go tool complains in these cases and it's enough to run `go mod tidy -compat=1.17` to solve the issue. It also puts quite some strain on the compiler- build times get slower and go-get takes a while to acquire the mat package whenever go-getting.

here is a demo of the effect of the mat import. https://github.com/soypat/mat-import-woes