Skip to content

Make Hessian sparsity detection work with SCT (prototype) #198

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

Conversation

gdalle
Copy link
Contributor

@gdalle gdalle commented May 1, 2025

This is just an example showing how to replace certain dense matrices with sparse ones to turn accidental zeros into structural zeros. This is enough to make the MWE of #193 work with SCT, but I haven't run wider tests, and I don't think you should merge this PR without a deeper reflection on structural sparsity handling

@franckgaga
Copy link
Member

franckgaga commented May 11, 2025

@gdalle
I would not merge this PR explicitly but another PR with similar ideas but implemented differently. Mainly: be more efficient with decision variables Z to input increments Δ U and input U conversions, but without explicitly using sparse-matrix.

Are you are of way to incorporate your ideas in a new pull request and explicitly mark you as a contributor ? The idea comes from you, so I would like you to appear as a contributor on the github page! Maybe just a dummy commit in the other PR?

@gdalle
Copy link
Contributor Author

gdalle commented May 11, 2025

Thanks for being so thoughtful about contributors!
Perhaps the easiest way would be for you to take this PR branch, add a second commit which undoes my first commit, and then be on your merry way to add more stuff?

@franckgaga
Copy link
Member

franckgaga commented May 11, 2025

Yes great idea, I'll do this.

edit: maybe not. Many commits that I wrote in JuliaControl:hessian_objective_debug branch are simply thrown away (since it is not possible to implement this feature with JuMP right now). So I would need to revert your commits and many of my commits before that. I think I'll use cherry picking.

@franckgaga franckgaga marked this pull request as ready for review May 11, 2025 17:26
@franckgaga
Copy link
Member

Closing in favor of #202 that uses cherry-picking

@franckgaga franckgaga closed this May 11, 2025
@gdalle
Copy link
Contributor Author

gdalle commented May 11, 2025

Just so you know, we're still planning on merging adrhill/SparseConnectivityTracer.jl#243, it's just a bit more complicated than we thought because it interferes with array-level overloads (essentially if we regard matrices or vectors of constants as black boxes in SCT, then we can't cherry-pick the non-zero coefficients)

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 this pull request may close these issues.

None yet

2 participants