Use @concrete for Tsit5Cache to hide non-dispatch type parameters#3652
Use @concrete for Tsit5Cache to hide non-dispatch type parameters#3652lo2545 wants to merge 3 commits into
Conversation
|
This isn't the same. The |
|
I've updated the approach. Instead of modifying the struct parameters |
| function Base.show(io::IO, cache::Tsit5Cache) | ||
| print(io, "Tsit5Cache{$(typeof(cache.u))}") | ||
| end | ||
|
|
||
| function Base.show(io::IO, ::MIME"text/plain", cache::Tsit5Cache) | ||
| print(io, "Tsit5Cache{$(typeof(cache.u))}") |
There was a problem hiding this comment.
This will have major invalidations so it's not recommended.
There was a problem hiding this comment.
Thank you for the feedback! Sorry. I understand that Base.show overrides
cause invalidations.
Could you give a hint on the correct approach for hiding non-dispatch
type parameters without breaking @cache and allocation-free performance?
Should I use a wrapper type like struct Opaque{T} val::T end,
or is there a pattern already used elsewhere in SciML for this?
There was a problem hiding this comment.
I don't think this will invalidate badly. The invalidation issue is show on types. changing showing of the type of a value is fine.
There was a problem hiding this comment.
If it's just the value, that doesn't do much though? The stacktrace and everything is untouched?
There was a problem hiding this comment.
stack traces are untouched, but this will clean up things like test failures that print the values.
There was a problem hiding this comment.
to be clear, I'm not saying this is necessarily a good idea, just that it's not an invalidation issue.
There was a problem hiding this comment.
Would overriding Base.show on the type itself fix the stack traces?
Base.show(io::IO, ::Type{<:Tsit5Cache}) = print(io, "Tsit5Cache")
Is this the right approach?
There was a problem hiding this comment.
no. that's the thing that causes endless invalidations.
There was a problem hiding this comment.
What is the recommended approach then?
Summary
Refactors
Tsit5Cacheto use@concretefromConcreteStructs.jlinstead of the manually parameterized
@cachestruct.Motivation
When users work with custom unit types (e.g.,
Unitful.jl), errorstack traces become unreadable due to type parameter pollution:
Before:
Tsit5Cache{Vector{Quantity{Float64, 𝐋 𝐓⁻¹}},
Vector{Quantity{Float64, 𝐋 𝐓⁻²}},
Vector{Float64},
typeof(trivial_limiter!),
typeof(trivial_limiter!),
Serial}
After:
Tsit5Cache
Changes
ConcreteStructs.jlas a dependency inProject.toml@cache struct Tsit5Cache{uType, rateType, ...}with@concrete struct Tsit5CacheTesting
All existing tests pass.