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

Non-dispatchable field specialization syntax. #45687

Open
ChrisRackauckas opened this issue Jun 15, 2022 · 14 comments
Open

Non-dispatchable field specialization syntax. #45687

ChrisRackauckas opened this issue Jun 15, 2022 · 14 comments

Comments

@ChrisRackauckas
Copy link
Member

I was looking again at #40138 and my more evolved understanding of this issue is the fact that fully parametric fields are treated in the same manner as other type parameters, and that just seems wrong. For example, if I have a type

struct MyType{T}
  x::T
end

What our current stacktrace assumes is that knowing T is "fundamental" to understanding MyType. In reality, I only did ::T because I wanted the Julia compiler to optimize it. If it wasn't a programming optimization, then I would've done:

struct MyType
  x
end

There's nothing fundamental to understanding the type of x that is required for understanding how MyType works: nothing ever dispatches on it, it's not part of the public API (so I don't intend anyone to ever dispatch on it), yet it's treated as first-class information. How can I allow program specialization here without allowing ease of dispatch? I can't. This seems to be a fundamentally missing feature in the type system.

For an example of this in practice, look at none other than the infamous ODEIntegrator type.

mutable struct ODEIntegrator{algType<:Union{OrdinaryDiffEqAlgorithm,DAEAlgorithm},IIP,uType,duType,tType,pType,eigenType,EEstT,QT,tdirType,ksEltype,SolType,F,CacheType,O,FSALType,EventErrorType,CallbackCacheType,IA} <: DiffEqBase.AbstractODEIntegrator{algType,IIP,uType,tType}
  sol::SolType
  u::uType
  du::duType
  k::ksEltype
  t::tType
  dt::tType
  f::F
  p::pType
  uprev::uType
  uprev2::uType
  duprev::duType
  tprev::tType
  alg::algType
  dtcache::tType
  dtchangeable::Bool
  dtpropose::tType
  tdir::tdirType
  eigen_est::eigenType
  EEst::EEstT
  qold::QT
  q11::QT
  erracc::QT
  dtacc::tType
  success_iter::Int
  iter::Int
  saveiter::Int
  saveiter_dense::Int
  cache::CacheType
  callback_cache::CallbackCacheType
  kshortsize::Int
  force_stepfail::Bool
  last_stepfail::Bool
  just_hit_tstop::Bool
  do_error_check::Bool
  event_last_time::Int
  vector_event_last_time::Int
  last_event_error::EventErrorType
  accept_step::Bool
  isout::Bool
  reeval_fsal::Bool
  u_modified::Bool
  reinitialize::Bool
  isdae::Bool
  opts::O
  destats::DiffEqBase.DEStats
  initializealg::IA
  fsalfirst::FSALType
  fsallast::FSALType
end

where one of its fields are:

mutable struct DEOptions{absType,relType,QT,tType,Controller,F1,F2,F3,F4,F5,F6,F7,tstopsType,discType,ECType,SType,MI,tcache,savecache,disccache}
  maxiters::MI
  save_everystep::Bool
  adaptive::Bool
  abstol::absType
  reltol::relType
  gamma::QT
  qmax::QT
  qmin::QT
  qsteady_max::QT
  qsteady_min::QT
  qoldinit::QT
  failfactor::QT
  dtmax::tType
  dtmin::tType
  controller::Controller
  internalnorm::F1
  internalopnorm::F2
  save_idxs::SType
  tstops::tstopsType
  saveat::tstopsType
  d_discontinuities::discType
  tstops_cache::tcache
  saveat_cache::savecache
  d_discontinuities_cache::disccache
  userdata::ECType
  progress::Bool
  progress_steps::Int
  progress_name::String
  progress_message::F6
  timeseries_errors::Bool
  dense_errors::Bool
  dense::Bool
  save_on::Bool
  save_start::Bool
  save_end::Bool
  save_end_user::F3
  callback::F4
  isoutofdomain::F5
  unstable_check::F7
  verbose::Bool
  calck::Bool
  force_dtmin::Bool
  advance_to_tstop::Bool
  stop_at_next_tstop::Bool
end

Beautiful. This is the thing that generated a literally 10,000 character stack trace when Unitful numbers were put into it. But why? Let's understand this in the context of the above.

  abstol::absType
  reltol::relType

abstol and reltol are parametrically typed because it's possible for a user to pass different types in there. Yes it's normal to solve(prob,Tsit5(),abstol=1e-6,reltol=1e-6), but you can also do solve(prob,Tsit5(),abstol=[1e-6,1e-2],reltol=1e-6) to have per-element tolerances. Great feature right? Well by allowing this feature, I now have to make the internal struct parametric, and so now everybody who uses the ODE solver has to see absType and relType in every stack trace. Mind you, not a single function in the entire library dispatches on this type parameter, nor is anything ever intended to, so this type parameter is a waste of time in any stacktrace: I've just learned to ignore it.

The integrator type has >100 type parameters at any given type, and the first thing we have to train a user to do is ignore all of them. What the hell? Don't we think something has gone very wrong here?

I think the issue is that we have in the type syntax conflated the different ideas of "I want this program to specialize on this field" and "I want to be able to dispatch on this field". When we write functions, we do f(x) and we know that f will (most likely, some heuristics) specialize on the type of x. We do not require that people write f(x::T) where T everywhere. If we did, our function stack traces would be ginormous because every call would describe methods like f(x::T,y::T2,z::T3) where {T1.T2.T3} and people would go "what does this mean?". But we do not treat types the same way: we force basically "where T" on every field that we want to specialize, and we print this information to the user that basically just say "hey, DifferentialEquations.jl likes to specialize, you wanted to know that right?"

No, nobody cares. So what can we do? What about a new language feature for non-dispatchable field specialization syntax?

mutable struct DEOptions{tType}
  maxiters::Specialize
  save_everystep::Bool
  adaptive::Bool
  abstol::Specialize
  reltol::Specialize
  gamma::Specialize
  qmax::Specialize
  qmin::Specialize
  qsteady_max::Specialize
  qsteady_min::Specialize
  qoldinit::Specialize
  failfactor::Specialize
  dtmax::tType
  dtmin::tType
  controller::Specialize
  internalnorm::Specialize
  internalopnorm::Specialize
  save_idxs::Specialize
  tstops::Specialize
  saveat::Specialize
  d_discontinuities::Specialize
  tstops_cache::Specialize
  saveat_cache::Specialize
  d_discontinuities_cache::Specialize
  userdata::Specialize
  progress::Bool
  progress_steps::Int
  progress_name::String
  progress_message::Specialize
  timeseries_errors::Bool
  dense_errors::Bool
  dense::Bool
  save_on::Bool
  save_start::Bool
  save_end::Bool
  save_end_user::Specialize
  callback::Specialize
  isoutofdomain::Specialize
  unstable_check::Specialize
  verbose::Bool
  calck::Bool
  force_dtmin::Bool
  advance_to_tstop::Bool
  stop_at_next_tstop::Bool
end

And it would then be really nice to have a macro in Base where this is equivalent to:

@specialize mutable struct DEOptions{tType}
  maxiters
  save_everystep::Bool
  adaptive::Bool
  abstol
  reltol
  gamma
  qmax
  qmin
  qsteady_max
  qsteady_min
  qoldinit
  failfactor
  dtmax::tType
  dtmin::tType
  controller
  internalnorm
  internalopnorm
  save_idxs
  tstops
  saveat
  d_discontinuities
  tstops_cache
  saveat_cache
  d_discontinuities_cache
  userdata
  progress::Bool
  progress_steps::Int
  progress_name::String
  progress_message
  timeseries_errors::Bool
  dense_errors::Bool
  dense::Bool
  save_on::Bool
  save_start::Bool
  save_end::Bool
  save_end_user
  callback
  isoutofdomain
  unstable_check
  verbose::Bool
  calck::Bool
  force_dtmin::Bool
  advance_to_tstop::Bool
  stop_at_next_tstop::Bool
end

This better captures the essence of what the type is, makes it easier to write code without performance bugs, and will go a very long way to making the stacktraces more legible.

@ChrisRackauckas
Copy link
Member Author

ChrisRackauckas commented Jun 15, 2022

Here is a concrete example from an actual user code I saw while giving a workshop this week:

https://gist.github.com/ChrisRackauckas/2861111912ec3b3f7ec4d7cd74e9567e

The stack trace is decreased from 15k characters to 7.5k characters without losing any real information.

Furthermore, if anyone did dispatch on these type parameters, I'd close their issue saying they were relying on non-public API information and so there code is brittle and I want nothing to do it it. These parameters are not just something that don't tend to be dispatch on, they are never intended to be dispatched on.

Today:

Screenshot 2022-06-15 032844

Tomorrow:

Screenshot 2022-06-15 033154

@vtjnash
Copy link
Sponsor Member

vtjnash commented Jun 15, 2022

Duplicate of #36201

@vtjnash vtjnash marked this as a duplicate of #36201 Jun 15, 2022
@vtjnash vtjnash closed this as completed Jun 15, 2022
@StefanKarpinski
Copy link
Sponsor Member

StefanKarpinski commented Jun 16, 2022

Since the primary issue here is stack traces being large, what about just not printing type parameters in stack traces that don't matter for dispatch of the method that's called? That would allow understanding what method was called and why without printing so much information in the stack trace and doesn't require any new language features either.

@vtjnash
Copy link
Sponsor Member

vtjnash commented Jun 16, 2022

What I think might also be interesting is to skip printing types that already appear elsewhere in the stacktrace

@StefanKarpinski
Copy link
Sponsor Member

I'm not sure I follow that idea.

@vtjnash
Copy link
Sponsor Member

vtjnash commented Jun 16, 2022

If you have f(x) = g(x) = h((x,)) = i((x,)) in the stacktrace somewhere, we don't really need to show the type of x more than once, since it could be assumed from context

@axsk
Copy link
Contributor

axsk commented Jun 23, 2022

Concerning readable stack traces: Often I am also interested only in functions in my current project.
A filter discarding the deeper calls would be nice to have.
Maybe this is something for OhMyREPL?

Edit: https://github.com/BioTurboNick/AbbreviatedStackTraces.jl seems to tackle this problem

@StefanKarpinski
Copy link
Sponsor Member

@vtjnash, that would seem to be more accurately described as not printing the type of values whose type has already been printed in the stack trace. The same type could be printed multiple times. Moreover, given that arguments don't generally have the same name or position all the way through the stack, that seems like it would be very confusing.

I'd like to focus on the core problem here: we print entire types in stack traces that are huge. The clearest most implementable solution is to limit what aspects of a type in a stack trace we print to only the parts of the type that are relevant to dispatch. This can be computed as a combination of concrete types of the arguments and the type of the method that is called. For example, if method is (AbstractArray, Integer) and the arguments are (Vector{SomeLargeType}, Int) then we could just print (Vector, Int). I'm going to open a new issue to discuss this specific idea.

@jonathan-laurent
Copy link

I like this proposal for reasons beyond getting more readable stacktraces.
To me, it also brings conceptual clarity. I often have to remind students that types serve two mostly orthogonal purposes in Julia: specialization and dispatch. This syntax proposal makes this distinction very clear.

Also, writing structs with ten type parameters gets tiring and I am sometimes tempted to leave performance on the table just to avoid writing this (as I had to do yesterday):

struct Tree{
    StateNodeArray,
    BoolNodeArray,
    Int16NodeArray,
    Float32NodeArray,
    BoolActionArray,
    Int16ActionArray,
    Float32ActionArray,
}
    state::StateNodeArray
    terminal::BoolNodeArray
    parent::Int16NodeArray
    valid_actions::BoolActionArray
    prev_action::Int16NodeArray
    prev_reward::Float32NodeArray
    prev_switched::BoolNodeArray
    policy_prior::Float32ActionArray
    value_prior::Float32NodeArray
    num_visits::Int16NodeArray
    total_values::Float32NodeArray
    children::Int16ActionArray
end

@StefanKarpinski
Copy link
Sponsor Member

Does it matter that some of the specialize only fields seems constrained to have the same type? This proposal doesn't offer a way to express that but it could. Eg I'm thinking of using a semicolon to separate the "real parameters" from the specialize only ones. Of course then you still have to give them a name, so maybe you could have another feature for omitting the name entirely (dare I suggest underscore?).

@ChrisRackauckas
Copy link
Member Author

Does it matter that some of the specialize only fields seems constrained to have the same type?

In the example I posted, the ones that are the same are only the same in order to decrease the number of type parameters and improve debugging. In fact, most of them are incorrect and we could specialize more if we set some to nothing some of the time to allow for reducing the number of arrays created if say saving isn't used. But that's not done because then the stack traces are effectively off the chart when using AD. So, 🤷‍♂️.

If necessary in a first implementation, one can just use an inner constructor.

@ChrisRackauckas
Copy link
Member Author

Reopened due to more discussions with @Keno. This discussion focused around the fact that it's not just stack traces but it also would make method matching less expensive. Basically what it comes down to is the following.

Those type parameters are thrown in the stack trace because they can be dispatched on. But, they weren't made to be dispatched on, they were made to specialize the type for performance. If someone dispatched on them, I would want to just slap them with an error that said "don't do that". So to me, this is pointing to two things accidentally being confused as one. Parametric typing and specialization are two separate features, they do two different things. They are just mixed here, but they do not necessarily need to be. You could specialize a type on types of fields without exposing the ability to dispatch on those field types to the user.

So privacy of the types is just one aspect, and probably not the most important. But allowing specialization without requiring dispatchability has multiple uses.

@ChrisRackauckas
Copy link
Member Author

With the printing improvements I think this can be closed.

@mauro3
Copy link
Contributor

mauro3 commented Jun 25, 2024

Whilst the stack-trace was the most notable expression of the problem, I do think that a distinction of "specializeable" vs "dispatchable" field-types could be a good language feature. So maybe leave this open, or write a new issue which focuses only on that?

@Keno Keno reopened this Jun 25, 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

No branches or pull requests

7 participants