Document fields of TreeSet

[blegat]

I needed to understand these from the implementation for the purpose of blegat/ArrayDiff.jl#19 so I thought I might as well document them now that I understand what they represent :)

[gdalle]

Note that these are internal structs which were never meant for outside consumption. If you need them to be public, can you give some more detail as to why?

[gdalle]

To clarify, more documentation, even for private objects, is always a good thing. I just wanna know if we can also make the public-facing API sufficient for your uses.

[blegat]

In JuMP, we compute the sparsity pattern of the Hessian of the objective I0, J0 and of each constraint Ik, Jk.
We then give the solver the concatenation vcat(I0, I1, ..., Im) and vcat(J0, J1, ..., Jm).
The, the solver give use a vector V which has the same length as the concatenation of the indices and we just need to fill it.
It seems the interface of SMC requires that we give a SparseMatrixCSC for the decompression:
https://github.com/gdalle/SparseMatrixColorings.jl/blob/31999f1409f5398ccd6224fb76f8dccdfe2d50c6/src/decompression.jl#L589
Instead, I would like for the objective and each constraint to just take a view of the corresponding part of V, and give it to SMC.
We can probably split this decompress! in two, one part that takes the nonzeros out of the SparseMatrixCSC and then redirects to another decompress! that just takes an AbstractVector of nonzeros and we we could use that second one from ArrayDiff

[gdalle]

Yeah that sounds feasible, but I'm more worried about the view part. Why do you need a view of V, and what kind of view?

[amontoison]

It needs a continuous view because JuMP is doing a coloring for the Hessian of the objective and then for the Hessian of each constraints.
They are not doing like ADNLPModels where we do directly a coloring of the Hessian of the Lagrangian.
After the computation of the nonzeros of each compressed Hessian, they need to do the decompression in the related part of the vector V to recover the sparse Hessian.

@blegat I am right?

[amontoison]

It is not right, i is not an edge index.

[blegat]

Should be fixed now

[amontoison]

We should keep this comment.
What do think of:

# reverse_bfs_orders will contain the reverse breadth first (BFS) traversal order for each tree in the forest concatenated

[blegat]

I simply moved it into the comment of the field of the struct. I feel that the definition of what reverse_bfs_orders will be should go there and here we should comment on why what we do achieve this

[blegat]

I added a new comment clarifying the confusion with the root of the Forest union-find datastructure that we thought we be useful during our discussion on the phone

[amontoison]

I don't think they have disjoint vertices. When the matrix is dense, we have n(n+1)/2 trees with two vertices (one for each edge) but only n vertices.

[blegat]

If a red vertex u is adjacent to both a blue and yellow vertex then the red-blue tree and red-yellow tree have the u vertex in common so they don't have disjoint vertices

[blegat]

Ahah, I contradicted myself in my reply. I meant disjoint edges indeed

[blegat]

Fixed it in the last commit

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (31999f1) to head (20ab117).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##              main      #296    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files           20        20            
  Lines         2027      2139   +112     
==========================================
+ Hits          2027      2139   +112     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.