Avoid broadcast in getfields/getproperties - #109
ChrisRackauckas-Claude wants to merge 4 commits into
Conversation
The previous getproperty./getfield. form over property-name tuples compiles broadcast/convert MethodInstances that Symbolics-style BroadcastStyle methods invalidate; rebuilding them under @recompile_invalidations can hit Julia's irinterp recursion limit. Use a generated getfields path (and ntuple for custom propertynames) instead. Co-authored-by: Cursor <cursoragent@cursor.com>
|
This is because the broadcast relies on type inference recursion, which is both slow and hits limits. So the library shouldn't rely on that. Discourse posts show breakdowns on types with large numbers of type parameters. |
|
cool, can you bump the version? |
|
Done |
|
Lets not touch the |
|
Super verbose comments, docs, typical claude :) Any chance this can be leaner?Also, whats a specific example that is fixed, before/after behavior?On Sep 9, 2026 06:32, Jan Weidner ***@***.***> wrote:jw3126 left a comment (JuliaObjects/ConstructionBase.jl#109)
Lets not touch the .md files
—Reply to this email directly, view it on GitHub, or unsubscribe.Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>
|
Co-authored-by: Christopher Rackauckas <accounts@chrisrackauckas.com>
Co-authored-by: Christopher Rackauckas <accounts@chrisrackauckas.com>
Okay, I removed the comments about why it's not broadcasting from the .md, but I kept the changes pointing people down a path that is easier on the compiler. Since I think a lot of people will just copy what the docs do, we might as well tell them to do
I made the comments just a quick line and a link back to the original issue that describes why this formulation is done. That should be small but sufficient enough for someone to understand the code. |
|
What does it actually fix, do you have some direct example showing difference before-after? |
Yes, see the OP. It has links to https://discourse.julialang.org/t/modelingtoolkit-internal-error-in-runtime/139291 With this, that is gone. Well, I also did the same thing downstream so that it's already fixed for users of SciML, but any similar type would also give a similar error right now on main.
Not sure what you mean. It's literally the exact same Julia code, just not using broadcast. |
Summary
getfieldsa@generatedexpansion ofgetfieldcalls, and route the commongetpropertiespath (whenpropertynames === fieldnames) through it.propertynames, build values with an explicitntupleofgetpropertyinstead ofgetproperty.(…).ntuplesketch).Why not broadcast?
The previous implementation was essentially:
Broadcasting over a property-name tuple (
NTuple{N,Symbol}or similar) compiles broadcast /convertMethodInstances on fully open object types. Those instances live in dependents' precompile caches (e.g. SciMLBaseremakeviaConstructionBase.getpropertiesonODEFunction). Loading packages that addBroadcastStylemethods — notably Symbolics — invalidates them;PrecompileTools.@recompile_invalidationsthen tries to rebuild them and can hit Julia'sduring ModelingToolkit precompilation (discourse #139291, SciMLBase #1595).
Semantics are unchanged; only the default implementation avoids those broadcast edges. SciMLBase #1595 is a scoped workaround for
AbstractSciMLFunction; this PR is the general upstream fix.Test plan
Pkg.test()on Julia 1.12.7 (full suite, including inference / no-alloc / custompropertynames/ StaticArrays / IntervalSets)Made with Cursor