Skip to content

Use @concrete for Tsit5Cache to hide non-dispatch type parameters#3652

Open
lo2545 wants to merge 3 commits into
SciML:masterfrom
lo2545:master
Open

Use @concrete for Tsit5Cache to hide non-dispatch type parameters#3652
lo2545 wants to merge 3 commits into
SciML:masterfrom
lo2545:master

Conversation

@lo2545
Copy link
Copy Markdown

@lo2545 lo2545 commented May 19, 2026

Summary

Refactors Tsit5Cache to use @concrete from ConcreteStructs.jl
instead of the manually parameterized @cache struct.

Motivation

When users work with custom unit types (e.g., Unitful.jl), error
stack 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

  • Added ConcreteStructs.jl as a dependency in Project.toml
  • Replaced @cache struct Tsit5Cache{uType, rateType, ...} with
    @concrete struct Tsit5Cache
  • All internal types remain fully concrete — no performance regression

Testing

All existing tests pass.

@ChrisRackauckas
Copy link
Copy Markdown
Member

This isn't the same. The @cache struct is used to populate the cache iterators for resize! definitions, and this new definition has many more type parameters than what's necessary (longer stack traceS).

@lo2545
Copy link
Copy Markdown
Author

lo2545 commented May 21, 2026

I've updated the approach. Instead of modifying the struct parameters
(which broke allocations and @cache), I added Base.show overrides that
display only the relevant uType to the user. All tests pass including
the allocation-free check.

Comment on lines +53 to +58
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))}")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will have major invalidations so it's not recommended.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's just the value, that doesn't do much though? The stacktrace and everything is untouched?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stack traces are untouched, but this will clean up things like test failures that print the values.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to be clear, I'm not saying this is necessarily a good idea, just that it's not an invalidation issue.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no. that's the thing that causes endless invalidations.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the recommended approach then?

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.

3 participants