API stability road map(s)

[Dan Kortschak]

I think that we are probably in a position where we can and need to
start thinking about about explicit API stability roadmaps and
statements for the gonum repositories.

I don't think that we can do this gonum-wide, but I do think we can do
this per-repo; different repos are at different states of development,
have different interaction dynamics (the lapack and blas packages have
tighter constraints and could probably have a promise made now), or may
already have implicit stability roadmaps (plot is an example of this I
think where there is a loose promise that the API will be stable by the
time plotinum goes off line at the end of the year).

Probably the best way to approach this is for the principal developers
for each repo to discuss what can and should be promised and when is an
issue leading to a github milestone and then bringing in others to flesh
out details.

Because of the potential diversity of API stability promises we also
probably need a single place to see this kind of information - to this
end a gonum.github.io webpage that describes gonum in a unified way,
linking to relevant docs is probably something we need to do.

Dan

[Brendan Tracey]

Here is my opinion on the packages:

BLAS -- blas/  blas/native blas/cgo are done. The only thing I would consider changing in blas64 are the constant inputs, for example swapping blas.Transpose and blas.Diag for booleans. Otherwise I am happy with the API.

LAPACK -- Same as for BLAS. The signatures are mostly constrained by the lapack functions.

Matrix:
- I would like to see the changes suggested in 38, moving many of the methods into functions (like Col)
- The Solve changes (suggested in 169)
- Do we still want maybe/maybefloat?
- Deleting of interfaces (138)
- Inverse should be a method, not a function (ala 38)
- Should we have a Vectorer interface instead of just *Vector (like the other types?). On the one hand, there's not much to be gained with the interface, but on the other hand it may help consistency if/when we add sparse support? I'm not sure.
- Dense.Norm should be a function (ala 38), but we should consider it taking in a constant type. Unlike floats.Norm, we don't support an arbitrary matrix norm, only a specific subset. Are the negative norms even matrix norms? Should they really be supported? (Just curious)
- We need to find how SVD/Eigen should be represented.
- Is there a better way to go from decomposition types into triangular? We have "LFrom", but there are different Ls, as in LU and LQ. Right now LFrom takes in a *LU, but that cannot support both types. There's an additional complication that the L's are different, in that sometimes they have unit diagonal, sometimes not
- SymDense rank two and rank one should take in *Vector, not []float64

Stat:
CovarianceMatrix should use *mat64.SymDense, not Dense
Otherwise I'm happy with the signatures.

dist/ distmv:
These need some more thought before guaranteeing stability I think. We could delete things and then guarantee stability, I suppose. Many of the functions are fine, but some of the derivative signatures are less clear. I know I'm the reason they are there (and I think the general functionality does have merit), but the signatures may need improving.

Related, but I've been thinking about bounds and gonum. Uniform is effectively one bound, and we explicitly represent []Bound in distmv.Uniform. This isn't the only place though. Optimize will eventually need bounds. I've been working on an lp solver, which also needs bounds. The next step is a QP solver, which also need bounds. Interop between all of these packages  is easier if there's just bound type. It's not clear where such a type would live. Maybe this is actually a use for general? Similarly, both convex and non-convex use linear constraints (equality and inequality). It would be nice to have this type defined in one place.

Floats:
- Delete AddConst?
- Delete EqualLength?
- We should match floats and mat64 with EqualApprox vs. EqualsApprox

Diff:
- I'm pretty happy with it.

Optimize:
I think we're getting close to the core set, but I don't know how Vladimir feels.

Root:
As is should be deleted. I have a long-standing commit there that I haven't thought about in a long time.

General:
As is, should be deleted

Unit:
I think the constants are, by and large, a bad idea. They add a lot to the API surface, and they don't scale to arbitrary types.

[Dan Kortschak]

On Fri, 2015-08-07 at 23:32 -0700, Brendan Tracey wrote:
> Here is my opinion on the packages:
>
> BLAS -- blas/ blas/native blas/cgo are done. The only thing I would
> consider changing in blas64 are the constant inputs, for example swapping
> blas.Transpose and blas.Diag for booleans. Otherwise I am happy with the
> API.

Agreed except that blas.Transpose and others are fixed by interop with
the cgo-backing implementation and transpose is a ternary value with H
being the third state. The other issue is that with the low level
packages I would like to keep the zero value for these as invalid for
safety reasons.

> LAPACK -- Same as for BLAS. The signatures are mostly constrained by the
> lapack functions.

Also agreed. We do need to have a caveat on the API stability promise
here that the Float64 interface is subject to change as functions are
added.

> Matrix:
> - I would like to see the changes suggested in 38, moving many of the
> methods into functions (like Col)
> - The Solve changes (suggested in 169)
> - Do we still want maybe/maybefloat?

Do we want to have optional errors? If not then no.

> - Deleting of interfaces (138)
> - Inverse should be a method, not a function (ala 38)
> - Should we have a Vectorer interface instead of just *Vector (like the
> other types?). On the one hand, there's not much to be gained with the
> interface, but on the other hand it may help consistency if/when we add
> sparse support? I'm not sure.
> - Dense.Norm should be a function (ala 38), but we should consider it
> taking in a constant type. Unlike floats.Norm, we don't support an
> arbitrary matrix norm, only a specific subset. Are the negative norms even
> matrix norms? Should they really be supported? (Just curious)

I just picked things that were provided by numpy.

> - We need to find how SVD/Eigen should be represented.
> - Is there a better way to go from decomposition types into triangular? We
> have "LFrom", but there are different Ls, as in LU and LQ. Right now LFrom
> takes in a *LU, but that cannot support both types. There's an additional
> complication that the L's are different, in that sometimes they have unit
> diagonal, sometimes not

Can we define an interface that gets this information across?

> - SymDense rank two and rank one should take in *Vector, not []float64
>
> Stat:
> CovarianceMatrix should use *mat64.SymDense, not Dense
> Otherwise I'm happy with the signatures.
>
> dist/ distmv:
> These need some more thought before guaranteeing stability I think. We
> could delete things and then guarantee stability, I suppose. Many of the
> functions are fine, but some of the derivative signatures are less clear. I
> know I'm the reason they are there (and I think the general functionality
> does have merit), but the signatures may need improving.

It's perfectly reasonable for us to have packages which are not subject
to stability promises due to being under development.

[Brendan Tracey]

On Saturday, August 8, 2015 at 1:22:05 AM UTC-6, kortschak wrote:

On Fri, 2015-08-07 at 23:32 -0700, Brendan Tracey wrote:
> Here is my opinion on the packages:
>
> BLAS -- blas/  blas/native blas/cgo are done. The only thing I would
> consider changing in blas64 are the constant inputs, for example swapping
> blas.Transpose and blas.Diag for booleans. Otherwise I am happy with the
> API.

Agreed except that blas.Transpose and others are fixed by interop with
the cgo-backing implementation and transpose is a ternary value with H
being the third state. The other issue is that with the low level
packages I would like to keep the zero value for these as invalid for
safety reasons.

Agree. I wouldn't change it in native/cgo. It's only if we wanted to change it in blas64.
 

> LAPACK -- Same as for BLAS. The signatures are mostly constrained by the
> lapack functions.

Also agreed. We do need to have a caveat on the API stability promise
here that the Float64 interface is subject to change as functions are
added.

Yep.
 

> Matrix:
> - I would like to see the changes suggested in 38, moving many of the
> methods into functions (like Col)
> - The Solve changes (suggested in 169)
> - Do we still want maybe/maybefloat?

Do we want to have optional errors? If not then no.

What do you mean by optional errors?
 

> - Deleting of interfaces (138)
> - Inverse should be a method, not a function (ala 38)
> - Should we have a Vectorer interface instead of just *Vector (like the
> other types?). On the one hand, there's not much to be gained with the
> interface, but on the other hand it may help consistency if/when we add
> sparse support? I'm not sure.
> - Dense.Norm should be a function (ala 38), but we should consider it
> taking in a constant type. Unlike floats.Norm, we don't support an
> arbitrary matrix norm, only a specific subset. Are the negative norms even
> matrix norms? Should they really be supported? (Just curious)

I just picked things that were provided by numpy.

> - We need to find how SVD/Eigen should be represented.
> - Is there a better way to go from decomposition types into triangular? We
> have "LFrom", but there are different Ls, as in LU and LQ. Right now LFrom
> takes in a *LU, but that cannot support both types. There's an additional
> complication that the L's are different, in that sometimes they have unit
> diagonal, sometimes not

Can we define an interface that gets this information across?

I'm not sure. "Factorizer" is a thing they all support, but doing so isn't generalizable to non mat64 types, and they don't have consistent behavior so it's not really a consistent abstraction. A reasonable option is to have "LFromLU", "LFromLQ", etc. It's more functions, but it's clear and compile time verifiable.
 

[Dan Kortschak]

On 09/08/2015, at 6:54 AM, "Brendan Tracey" <tracey....@gmail.com> wrote:

Agreed except that blas.Transpose and others are fixed by interop with
the cgo-backing implementation and transpose is a ternary value with H
being the third state. The other issue is that with the low level
packages I would like to keep the zero value for these as invalid for
safety reasons.

Agree. I wouldn't change it in native/cgo. It's only if we wanted to change it in blas64.

I think that unnecessarily specialises things - remember that we support complex blas at that level too.

> - Do we still want maybe/maybefloat?

Do we want to have optional errors? If not then no.

What do you mean by optional errors?

The original design intention was to allow people to wrap potentially erroring functions with the maybe functions and have the panics that we make detectable. This is the reason for the error type and also why there are some panics which explicitly pass a string rather than a mat64.Error.

Do we want that entire mechanism to go away? I don't use it, but I think it has utility given that we are pretty hard on the use of panic.

[Brendan Tracey]

On Saturday, August 8, 2015 at 3:34:43 PM UTC-6, kortschak wrote:

On 09/08/2015, at 6:54 AM, "Brendan Tracey" <tracey....@gmail.com> wrote:

Agreed except that blas.Transpose and others are fixed by interop with
the cgo-backing implementation and transpose is a ternary value with H
being the third state. The other issue is that with the low level
packages I would like to keep the zero value for these as invalid for
safety reasons.

Agree. I wouldn't change it in native/cgo. It's only if we wanted to change it in blas64.

I think that unnecessarily specialises things - remember that we support complex blas at that level too.

Great. I just wanted to put forward the idea before we fix it forever.
 

> - Do we still want maybe/maybefloat?

Do we want to have optional errors? If not then no.

What do you mean by optional errors?

The original design intention was to allow people to wrap potentially erroring functions with the maybe functions and have the panics that we make detectable. This is the reason for the error type and also why there are some panics which explicitly pass a string rather than a mat64.Error.

Do we want that entire mechanism to go away? I don't use it, but I think it has utility given that we are pretty hard on the use of panic.

I write code where it's okay to panic, so I don't fully understand the needs of those where it can. That said, I don't see the use case for wrapping an individual mat64 call with Maybe. We only panic on programmer error, so if there was a specific call I thought was wrong I'd try to ensure it was correct (say, by checking the sizes and  return an error). In general, if I had a service that couldn't panic and use mat64, I'd wrap the entire algorithm in a recover statement, instead of attempting to wrap each mat64 call (which wouldn't be sufficient to protect other kinds of panics).

[Dan Kortschak]

OK, so we remove the maybes and add some docs to Error explaining why it exists.

[Dan Kortschak]

I was looking at this last night, and I think ErrMismatch could be renamed to MethodMismatch.

[Dan Kortschak]

On Fri, 2015-08-07 at 23:32 -0700, Brendan Tracey wrote:

> BLAS -- blas/ blas/native blas/cgo are done. The only thing I would
> consider changing in blas64 are the constant inputs, for example
> swapping blas.Transpose and blas.Diag for booleans. Otherwise I am
> happy with the API.
>
> LAPACK -- Same as for BLAS. The signatures are mostly constrained by
> the lapack functions.

Here are proposed promises for these two
https://gist.github.com/kortschak/e64da94971f3ae2c4046

We probably want to sort out the constants in gonum/lapack though.

[Brendan Tracey]

The general document looks good.

There are two bigger questions though
1)  Do we still like that all of the gonum libraries are under different git repositories? It was nice while starting development, but as the connectivity gets larger it may be nice to be able to make changes in different repos in one commit (such as add mat64 functionality and use that functionality in blas).

2) I have gonum.org registered. Do we want to change our import paths to be there? I'd need help setting it up (figuring out where to host it, etc.), but then we could guarantee stability at gonum.org/blas or gonum.org/code/blas.

[Dan Kortschak]

On 10/08/2015, at 4:51 AM, "Brendan Tracey" <tracey....@gmail.com> wrote:

> 1) Do we still like that all of the gonum libraries are under different git repositories? It was nice while starting development, but as the connectivity gets larger it may be nice to be able to make changes in different repos in one commit (such as add mat64 functionality and use that functionality in blas).

I do. Single commit changes are thing that's nice where the central change is breaking behaviour, but that won't be an issue after the imposition of an API stability.

Personally I think small repos with tightly defined functionality work better.

> 2) I have gonum.org registered. Do we want to change our import paths to be there? I'd need help setting it up (figuring out where to host it, etc.), but then we could guarantee stability at gonum.org/blas or gonum.org/code/blas.

This is a pretty big move, but something that is probably worth while - I think we need agreement from everyobody. The hosting can remain at github.

[Sebastien Binet]

FWIW, you have my +1.

the wording for the api compat looks fine and sensible to me.
one thing though: you omitted the part about "Struct literals" in the
api stability exception section (gokit's rfc007 has it).
is it on purpose?
it seems to me like a useful exception to make to not paint ourself in a corner.

-s

[Dan Kortschak]

This was for a reason, and part of the reason for keeping separate repos.

In the case of lapack and blas the struct types have no reason to change since they reflect an API that has remained stable for decades. I think we are pretty safe omitting that exception in this case (but would happily change if people think this is foolhardy). I think this omission would only exist for blas and lapack.

[Chris Tessum]

I have a version of Unit that has the constant types deleted (if that's what you mean by constants) and some other changes made: http://godoc.org/github.com/ctessum/unit (the package doc isn't updated though).  I was thinking that this could be combined with a build tag (like `+build nocheckunits` or something like that) that switched to a version that doesn't check units so that, in cases where the units don't depend on user input, one could do development with units checking and then running in production without units checking to avoid the runtime overhead. The problem I see with compile-time unit checking is I don't see a way to check units for divide and multiply operations. 

[Vladimír Chalupecký]

Optimize:
I think we're getting close to the core set, but I don't know how Vladimir feels.

Yes, I agree that we are almost there, I like what we have. There are some points that I think we should consider (some we have discussed before, some not):

* Meaning or interpretation of the various iteration types and what actions we should take for each of them.

* I know that I proposed the Needs() method, but I am not very happy with it and I think/hope that we could get rid of it.

The optimizers already announce what they need through EvaluationType, so we can allocate upon request in evaluate().

For the initial data we could take them from Settings if provided by the client and InitialIteration has not yet been announced.

Maybe there is some issue with this but I feel it might work.

* Location.Hessian should be mat64.Symmetric instead of *SymDense so that in the future Hessian could be sparse. But then the question arises who allocates the concrete type. Methods in Init()? Some other way?

Root:

As is should be deleted. I have a long-standing commit there that I haven't thought about in a long time.

Agreed, delete it. With our experience from optimize we can create a very nice root package.

Just my two cents.

Vladimir

[Brendan Tracey]

On Aug 20, 2015, at 9:55 PM, Vladimír Chalupecký <vladimir....@gmail.com> wrote:

Optimize:
I think we're getting close to the core set, but I don't know how Vladimir feels.

Yes, I agree that we are almost there, I like what we have. There are some points that I think we should consider (some we have discussed before, some not):

* Meaning or interpretation of the various iteration types and what actions we should take for each of them.

* I know that I proposed the Needs() method, but I am not very happy with it and I think/hope that we could get rid of it.

The optimizers already announce what they need through EvaluationType, so we can allocate upon request in evaluate().

The thing about this is that it could possibly fail after several function evaluations, but the check should be as soon as possible. A method may want to do several function evaluations before doing a gradient evaluation. 

For the initial data we could take them from Settings if provided by the client and InitialIteration has not yet been announced.

Maybe there is some issue with this but I feel it might work.

* Location.Hessian should be mat64.Symmetric instead of *SymDense so that in the future Hessian could be sparse. But then the question arises who allocates the concrete type. Methods in Init()? Some other way?

It can’t just be Symmetric. At least it has to be a SymRankOner and a RankTwoer. Given that, it’s not clear to me that it should be generic. Will the matrix be kept sparse after many RankTwo updates? Even if it can, we already have the sparse BFGS version — that’s what LBFGS is.

Root:

As is should be deleted. I have a long-standing commit there that I haven't thought about in a long time.

Agreed, delete it. With our experience from optimize we can create a very nice root package.

Just my two cents.

Vladimir

--
You received this message because you are subscribed to the Google Groups "gonum-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to gonum-dev+...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Vladimír Chalupecký]

* I know that I proposed the Needs() method, but I am not very happy with it and I think/hope that we could get rid of it.

The optimizers already announce what they need through EvaluationType, so we can allocate upon request in evaluate().

The thing about this is that it could possibly fail after several function evaluations, but the check should be as soon as possible. A method may want to do several function evaluations before doing a gradient evaluation. 

And when it requests a gradient evaluation and Location.Gradient is nil, then it would be allocated before doing the evaluation. But it was really just an idea for considering, the case for this is not so strong. LinesearchMethod would have to be changed because it uses non-nillness of Location's fields for deciding the next evaluation type. And then there is the issue of the initial data. 

For the initial data we could take them from Settings if provided by the client and InitialIteration has not yet been announced.

Maybe there is some issue with this but I feel it might work.

* Location.Hessian should be mat64.Symmetric instead of *SymDense so that in the future Hessian could be sparse. But then the question arises who allocates the concrete type. Methods in Init()? Some other way?

It can’t just be Symmetric. At least it has to be a SymRankOner and a RankTwoer. Given that, it’s not clear to me that it should be generic. Will the matrix be kept sparse after many RankTwo updates? Even if it can, we already have the sparse BFGS version — that’s what LBFGS is.

Location.Hessian is the Hessian of the objective function and we do not touch it, so it can be just Symmetric, there is no relation to BFGS. If it were sparse, the linear solve in Newton could make use of the fact, for example.

[Brendan Tracey]

On Aug 20, 2015, at 11:26 PM, Vladimír Chalupecký <vladimir....@gmail.com> wrote:

* I know that I proposed the Needs() method, but I am not very happy with it and I think/hope that we could get rid of it.

The optimizers already announce what they need through EvaluationType, so we can allocate upon request in evaluate().

The thing about this is that it could possibly fail after several function evaluations, but the check should be as soon as possible. A method may want to do several function evaluations before doing a gradient evaluation. 

And when it requests a gradient evaluation and Location.Gradient is nil, then it would be allocated before doing the evaluation. But it was really just an idea for considering, the case for this is not so strong. LinesearchMethod would have to be changed because it uses non-nillness of Location's fields for deciding the next evaluation type. And then there is the issue of the initial data. 

For the initial data we could take them from Settings if provided by the client and InitialIteration has not yet been announced.

Maybe there is some issue with this but I feel it might work.

* Location.Hessian should be mat64.Symmetric instead of *SymDense so that in the future Hessian could be sparse. But then the question arises who allocates the concrete type. Methods in Init()? Some other way?

It can’t just be Symmetric. At least it has to be a SymRankOner and a RankTwoer. Given that, it’s not clear to me that it should be generic. Will the matrix be kept sparse after many RankTwo updates? Even if it can, we already have the sparse BFGS version — that’s what LBFGS is.

Location.Hessian is the Hessian of the objective function and we do not touch it, so it can be just Symmetric, there is no relation to BFGS. If it were sparse, the linear solve in Newton could make use of the fact, for example.

Yep, you’re totally right. Sorry.

[Dan Kortschak]

On Sun, 2015-08-09 at 21:41 +0000, Dan Kortschak wrote:
> > 2) I have gonum.org registered. Do we want to change our import
> paths to be there? I'd need help setting it up (figuring out where to
> host it, etc.), but then we could guarantee stability at
> gonum.org/blas or gonum.org/code/blas.
>
> This is a pretty big move, but something that is probably worth while
> - I think we need agreement from everyobody. The hosting can remain at
> github.
>

Do we have a community view on this? I would like to be able to make API
promises for matrix, blas and lapack by the end of the year - blas and
lapack could be done now IMO with the exception of this, matrix's
promise is dependent on heroic changes being made by Brendan, but also
this.

So far we have three voices: Brendan, Seb and me. Seb and I have made a
positive vote and Brendan was promulgating the idea, so I imagine there
is an implicit vote there. Any others?

Dan

[Dan Kortschak]

Following this up.

I think they should stay on the basis that there is no reason that
individual calls need to be wrapped, and the Maybe function can be used
to do the entire algorithm wrapping in a way that distinguishes
client-facing matrix errors from other panics that we or others do
inside the wrapped code. This is true for algorithms that return
nothing, but analogous systems can be built with MaybeFloat and
MaybeComplex for float64 and complex128.

[Dan Kortschak]

On Sun, 2015-08-09 at 08:39 +0930, Dan Kortschak wrote:
> Here are proposed promises for these two
> https://gist.github.com/kortschak/e64da94971f3ae2c4046
>

Updated to include the matrix repo's promise. Can we have comments on
these prior to a public (golang-nuts etc) notice that we plan to have an
API promise in the next couple of months and that after that backwards
incompatible changes will be restricted to the cases listed.

Dan

[Vladimír Chalupecký]

You can add my positive vote.

Vladimir 

[Sebastien Binet]

ditto.

-s

[Brendan Tracey]

BLAS / LAPACK: We should probably explicitly note that when silces have NaN in them, function behavior is unspecified. The discussion in https://github.com/xianyi/OpenBLAS/issues/624 illuminates how tricky it is to actually specify something that all platforms will agree on.

Do we want to say something about printing and specific strings in exported constants not being covered? Aka if we want to change the formatting in mat64.Format we have the right.

[Dan Kortschak]

Both of these sound reasonable.