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

More concise printing of detector type? #189

Closed
gdalle opened this issue Sep 5, 2024 · 3 comments · Fixed by #190
Closed

More concise printing of detector type? #189

gdalle opened this issue Sep 5, 2024 · 3 comments · Fixed by #190

Comments

@gdalle
Copy link
Collaborator

gdalle commented Sep 5, 2024

I'm wondering if we can make the printing of TracerSparsityDetector() shorter. At the moment it looks like

julia> TracerSparsityDetector()
TracerSparsityDetector{SparseConnectivityTracer.GradientTracer{SparseConnectivityTracer.IndexSetGradientPattern{Int64, BitSet}}, SparseConnectivityTracer.HessianTracer{SparseConnectivityTracer.DictHessianPattern{Int64, BitSet, Dict{Int64, BitSet}, SparseConnectivityTracer.NotShared}}}()

But we don't have to specify all type parameters if they correspond to the default settings. You can draw inspiration from the new printing I coded for ADTypes, which only tries to reproduce the constructor. See example here:
https://github.com/SciML/ADTypes.jl/blob/f206682addbc27b450f51d51e9a4082fb9b7cdb4/src/dense.jl#L190-L196

@adrhill
Copy link
Owner

adrhill commented Sep 5, 2024

Yes, that would be an improvement.

@adrhill
Copy link
Owner

adrhill commented Sep 5, 2024

How about this?

julia> for detector in (:TracerSparsityDetector, :TracerLocalSparsityDetector)
           @eval function Base.show(io::IO, d::$detector{GradientTracer{PG}, HessianTracer{PH}}; indent=0) where {PG,PH}
               println(io, $detector, "{")
               println(io, "  ", "GradientTracer pattern: ", PG)
               println(io, "  ", "HessianTracer pattern:  ", PH)
               print(io, "}")
               return nothing
           end
       end

julia> TracerSparsityDetector()
TracerSparsityDetector{
  GradientTracer pattern: SparseConnectivityTracer.IndexSetGradientPattern{Int64, BitSet}
  HessianTracer pattern:  SparseConnectivityTracer.DictHessianPattern{Int64, BitSet, Dict{Int64, BitSet}, SparseConnectivityTracer.NotShared}
}

Or should we just print TracerSparsityDetector() instead of leaking all our internals?

@gdalle
Copy link
Collaborator Author

gdalle commented Sep 5, 2024

Ideally, Base.show(io, object) is supposed to give you a string that you can parse back into the object itself. Of course not everyone knows or cares.
In any case, I don't think most users will ever go beyond TracerSparsityDetector(), and those users don't need to know the type parameters. So for them, I would simplify printing of at least the default detector, and maybe leave complex printing if one of the settings differs from the default?

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