Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Documentation needs to be added #293

Open
mattsignorelli opened this issue Jul 16, 2024 · 8 comments
Open

Documentation needs to be added #293

mattsignorelli opened this issue Jul 16, 2024 · 8 comments

Comments

@mattsignorelli
Copy link

It is an uphill battle figuring out how to implement this interface. The only documentation I can find on how to do so is hidden in this paper, but even that I find difficult to understand. There needs to be a step-by-step "for dummies" guide on how exactly to implement this interface on the Documenter site, covering everything I need to do

@odow
Copy link
Member

odow commented Jul 16, 2024

MutableArithmetics is not something that we necessarily expect third-parties to widely implement. What is the motivation and where would it be used?

@mattsignorelli
Copy link
Author

What is the motivation and where would it be used?

GTPSA.jl (which I maintain) and ADOLC.jl are both automatic differentiation libraries where the number types are mutable (because they wrap C/C++ libraries). And also bmad-sim/GTPSA.jl#113 .

MutableArithmetics is not something that we necessarily expect third-parties to widely implement.

Why not? Isn't this point of an interface package?

@nsajko
Copy link
Contributor

nsajko commented Jul 16, 2024

FWIW, this is an incomplete list of function to which methods may be added by user packages, as far as I know:

  • buffer_for (documented in the README, but currently not in the docs, xref doc: buffer_for isn't documented #246): needed for the buffered_* functions
  • buffered_operate!
  • buffered_operate_to!
  • mutability
  • mutable_copy
  • operate
  • operate!
  • operate_to!

@odow
Copy link
Member

odow commented Jul 17, 2024

Why not? Isn't this point of an interface package?

Because it requires making the type a <: MA.AbstractMutable, and it requires using MA.@rewrite or the low-level API.

This really makes sense only if MA is used as part of some larger interface like JuMP. Just implementing this interface won't make user-code faster.

I don't know if I understand why this would be useful for ADOLC.

@nsajko
Copy link
Contributor

nsajko commented Jul 17, 2024

it requires making the type a <: MA.AbstractMutable

That's not necessary when defining mutability, right?

Just implementing this interface won't make user-code faster.

The way I see it, MA provides a single interface that's generic enough to be suitable for avoiding unnecessary allocation with all mutable types. A single interface is pretty great as opposed to an unlimited number of such designs, because then users of these (optionally) mutating operations don't have to learn a new interface for each package, and it allows better code composition.

@odow
Copy link
Member

odow commented Jul 17, 2024

Perhaps I'll let @blegat speak to whether it is a good fit because this package is really his.

But note that MA is not really a generic interface for all mutable types. It's really specialized for the types of operations that occur when building linear and quadratic expressions, such as those in JuMP or the various polynomial packages.

@mattsignorelli
Copy link
Author

Yes all I need to do is define the mutability trait.

I don't know if I understand why this would be useful for ADOLC.

While I'm not super familiar with ADOLC, on the surface it seems very similar to GTPSA; a Julia interface to a C-based automatic differentiation library. In this case, the number types are mutable. From this alone, one could define a buffer of preallocated number types and when temporaries in an expression are needed, use from that buffer. GTPSA goes further in that many operations also allow aliasing (e.g. add!(z,z,z) or mul!(z,x,z)), which means even less temporaries being generated.

Currently in GTPSA, the mutability of the truncated power series in GTPSA TPSs can be taken advantage of in this way I describe above by prepending the macro @FastGTPSA to expressions. In a simple example, there is a 2x speedup. However this current approach is very hacky, and while the macro has no impact on number types other than TPS, it certainly is far from generic. Unfortunately overloading the equal sign in Julia is also not an option, so we are stuck with having some kind of macro.

The best solution, in my opinion, would be to generalize @rewrite to work for any function defined using the MA interface (not just basic arithmetic) and let that replace @FastGTPSA. This makes the code much more type-generic, and also would help future package developers who run into the same problem as me have an immediate solution, just by satisfying MA.

@blegat
Copy link
Member

blegat commented Jul 22, 2024

The lack of documentation is a known issue, @kalmarek had the same comment in #286

The AbstractMutable abstract type is useful if you want the src/dispatch.jl method overload to redirect linear algebra operations to the implementation in MA instead of LinearAlgebra and SparseArrays that are quite slow since they don't use the MA API. It's not mandatory to be a subtype of AbstractMutable though.

If you're only interested in allowing your type MyType to be used in generic algorithm that uses the operate!! API so that it works on non-mutable and mutable types then you need to implement:

  • MA.mutability(::Type{MyType}) = IsMutable()
  • MA.promote_operation(::typeof(f), ::Type{MyType}, ::Type{MyType} = MyType
  • MA.operate!(::typeof(f), ::MyType, ::MyType) and MA.operate_to!(::MyType, ::typeof(f), ::MyType, ::MyType)

The set of f for which you want to do this depends on what you want to do. You can do it the lazy way and try to implement the minimum and only implement an operate! or operate_to! method when you get a MethodError. Because mutability returns IsMutable, there will be no fallback to non-mutating method, you are guaranteed to get a MethodError so no silent inefficiency :)

I think a tutorial in the docs should help, I'll try to find time to do this.

Feel free to ask question in this thread as well, it will give me an idea of what kind of documentation is needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants