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

Export sparsity detection interface #82

Closed
adrhill opened this issue Sep 4, 2024 · 6 comments · Fixed by #83
Closed

Export sparsity detection interface #82

adrhill opened this issue Sep 4, 2024 · 6 comments · Fixed by #83

Comments

@adrhill
Copy link
Contributor

adrhill commented Sep 4, 2024

The interface for sparsity detection should be exported, or at least marked as public.
The exported symbols are very unlikely to cause name collisions:

  • jacobian_sparsity
  • hessian_sparsity
  • AbstractSparsityDetector
  • NoSparsityDetector

The same applies to the matrix coloring interface as well.

@ChrisRackauckas
Copy link
Member

sounds fine to me.

@adrhill
Copy link
Contributor Author

adrhill commented Sep 4, 2024

I'll open a PR if @gdalle doesn't object.

@gdalle
Copy link
Collaborator

gdalle commented Sep 4, 2024

Marked as public, definitely, but that would require bringing Compat as an additional dependency since the public keyword only lands in Julia 1.11.

Exports are essentially useless because they don't semantically denote public API, they just clutter the namespace. I would be okay with exporting the first two, but no one is ever gonna use the latter two in their code so I'm firmly against exporting those.

@gdalle
Copy link
Collaborator

gdalle commented Sep 4, 2024

On second thought it's not a great idea to export jacobian_sparsity and hessian_sparsity because Symbolics also defines functions with the same name (which are not just methods for the ADTypes functions). In other words, if Symbolics ever decides to export them, the code of anyone who wrote

using ADTypes, Symbolics

will be broken

@ChrisRackauckas
Copy link
Member

Symbolics should extend.

@gdalle
Copy link
Collaborator

gdalle commented Sep 4, 2024

Unclear because the Symbolics functions apply to different data types (symbolic variables and expressions)

@gdalle gdalle closed this as completed in #83 Sep 9, 2024
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.

3 participants