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

Allow more Hessian set types than just Set{Tuple{Int,Int}} #105

Closed
gdalle opened this issue May 24, 2024 · 3 comments · Fixed by #103
Closed

Allow more Hessian set types than just Set{Tuple{Int,Int}} #105

gdalle opened this issue May 24, 2024 · 3 comments · Fixed by #103

Comments

@gdalle
Copy link
Collaborator

gdalle commented May 24, 2024

At the moment, using TracerSparsityDetector{G,H} with H something other than Set{Tuple{Int,Int}} will fail. But one might like to try:

  1. G = MySet{Int} and H = MySet{Tuple{Int,Int}} (definitely)
  2. G = MySet{Int} and H = MyOtherSet2Tuple{Int,Int}} (maybe)

To remedy this, here are the steps to take:

  • Allow MySet to accept things beyond Number (easy)
  • Implement union! for MySet (easy except for RecursiveSet)
  • Make sure that the result of the Cartesian product x(::MySet1{Int}, ::MySet1{Int}) can be union-ed with MySet{Tuple{Int,Int}}. This can be solved in two non-mutually-exclusive ways:
    • Make x(::MySet{Int}, ::MySet{Int}) return a MySet{Tuple{Int,Int}}. That way, the existing homogeneous union(::MySet{T}, ::MySet{T}) will work
    • Implement heterogeneous unions union(::MySet{T}, ::AbstractSet{T}). I don't like this option because it will presumably be very slow, in a way that is not directly visible to the user.
@adrhill
Copy link
Owner

adrhill commented May 24, 2024

I've spent a lot of time thinking about this and think that the cleanest solution is to implement several methods

×(::Type{H}, grad1::G, grad2::G)

for HessianTracer{G,H} / TracerSparsityDetector{G,H}.

It solves options 1 and 2 and avoids some of the pitfalls of heterogeneous Hessian-set unions, since H is specified prior to computing a Hessian-set union.

@gdalle
Copy link
Collaborator Author

gdalle commented May 24, 2024

okay, but I'll start with the homogeneous methods

@adrhill
Copy link
Owner

adrhill commented May 24, 2024

And for maximum performance, we might even end up needing

×!(hessian_buffer::H, grad1::G, grad2::G)

in the long term to avoid allocations.

@gdalle gdalle linked a pull request May 24, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants