ChrisRackauckas
commented
8 months ago This is completely changing with the v9 so after the update please rerun this and it should be fixed. @AayushSabharwal make note.
Closed aditya-sengupta closed 8 months ago
ChrisRackauckas
commented
8 months ago This is completely changing with the v9 so after the update please rerun this and it should be fixed. @AayushSabharwal make note.
AayushSabharwal
commented
8 months ago This works with v9, once https://github.com/SciML/ModelingToolkitStandardLibrary.jl/pull/271 is merged and the common defintion of t is used instead of z
aditya-sengupta
commented
8 months ago Is it necessary to use the common definition of t specifically, or can I still declare my own independent variable?
On Thu, 22 Feb 2024, 04:33 Aayush Sabharwal, @.***> wrote:
This works with v9, once SciML/ModelingToolkitStandardLibrary.jl#271 https://github.com/SciML/ModelingToolkitStandardLibrary.jl/pull/271 is merged and the common defintion of t is used instead of z
— Reply to this email directly, view it on GitHub https://github.com/SciML/ModelingToolkit.jl/issues/2475#issuecomment-1959365685, or unsubscribe https://github.com/notifications/unsubscribe-auth/AIK476QNCOI5RDT6DCC3LVTYU43KBAVCNFSM6AAAAABDSBXMGOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNJZGM3DKNRYGU . You are receiving this because you authored the thread.Message ID: @.***>
ChrisRackauckas
commented
8 months ago All equations must use the same independent variable. For the standard library, its equations are all defined using the common definition of t (and actually one with units!)
aditya-sengupta
commented
8 months ago If I'm solving an ODE over space instead of time (also with units) is there a recommended way to swap this out? t doesn't conflict with anything but I'm interested in steady-state solutions so it's also not as useful as the v8 SampledData.
In any case, thanks for the help, I'll close this after I've got v9 and confirmed this.
AayushSabharwal
commented
8 months ago I don't think there's a way to swap this out. With the v8 standard library, it doesn't really matter since t is unitless, but we'll soon have one with units. This could also be separated out into a math block that the source then relies on. That said, you could probably get something working immediately using DataInterpolations.jl and Blocks.StaticNonLinearity from the standard library
aditya-sengupta
commented
8 months ago What would be the issue with letting the user pass in the independent variable and letting that have whatever dimensions make sense? Locking users in to just integrating along time feels restrictive - could you elaborate on "a math block that the source then relies on"?
aditya-sengupta
commented
8 months ago I'm also not sure about this, but I think StaticNonLinearity + DataInterpolations would run into a similar issue to what I had initially, where swapping out the data needs the system to be remade. Is that accurate, or would this way work the same as SampledData?
AayushSabharwal
commented
8 months ago could you elaborate on "a math block that the source then relies on"?
Math blocks (src/Blocks/math.jl) are unitless transformations where the input and output can be connected to. Source blocks (src/Blocks/source.jl) are somewhat similar, except the input is always time. The standard library could likely be updated so the source blocks use the corresponding math blocks.
I just looked a bit deeper into the source code and realized this doesn't solve your problem, though 😅 since math blocks also have time-dependent inputs and outputs.
The StaticNonLinearity + DataInterpolations would also run into this issue I guess. The standard library Blocks need to support swapping out the independent variable for your case to work, which they don't currently.
aditya-sengupta
commented
8 months ago Based on looking at the SampledData source, I think I'd be able to make a component that's almost the same but using my library's z instead, by switching out t in a few places like this: https://github.com/SciML/ModelingToolkitStandardLibrary.jl/blob/2da69d3a953bc3f291321d0abce09bdd8594a342/src/Blocks/sources.jl#L670
Sounds like it'd be hard to make this work as a standard library solution immediately, but as a workaround for me does that sound right?
AayushSabharwal
commented
8 months ago That should work
AayushSabharwal
commented
8 months ago Can this issue be closed now?
ChrisRackauckas
commented
8 months ago Reopen if there's something that comes up and has a reproducer, indeed this is completed to our knowledge.
aditya-sengupta
commented
8 months ago This persists when using z instead of t and my version of SampledData. I modified the SampledData functions by changing them to SampledDataZ and changing each z to t, here: https://pastebin.com/rzMndnw4
If I use t and D from ModelingToolkit as the independent variable and derivative, and the SampledData block from the standard library, it works (https://pastebin.com/qMsFyu8f), but if I use SampledDataZ and my own independent variables (@variables z, Dz = Differential(z)) it throws the same error as before.
I've inspected both the version with t and with z and it seems to do the same thing, so structural_simplify must be treating t in a unique way. But I can't find where in the source it could be doing that, since this seems to be happening somewhere deep in the tearing logic.
using Symbolics
using ModelingToolkit
using ModelingToolkitStandardLibrary
using ModelingToolkitStandardLibrary.Blocks: SampledData
using MTKCompat # where SampledDataZ, z, Dz live
@variables T(z) P(z) # "background" physical variables: want to use a SampledData block for them
# so we can swap out what physics scenario we're looking at easily/without recompiling the system.
@variables qt(z) # physical variable we're solving for
# making data using SampledData blocks
z_range = 8e8
num_steps = 101
dz_val = z_range / (num_steps - 1)
z_val = collect(0:dz_val:8e8)
sdata_P = SampledDataZ(1e8 * exp.(-z_val / 8e7), dz_val; name=:P)
@named model_one_data = ODESystem([
P ~ sdata_P.output.u,
Dz(qt) ~ min(0.0, min(qt, 8.29e7 / P) - qt),
], z, [qt, P], [];
systems = [sdata_P]
)
s = structural_simplify(model_one_data) # works
sdata_T = SampledDataZ(2e3 * ones(Int(num_steps)), dz_val; name=:T)
@named model_two_data = ODESystem([
T ~ sdata_T.output.u,
P ~ sdata_P.output.u,
Dz(qt) ~ min(0.0, min(qt, 10^(13.61 - 11382.0 / T) / P) - qt),
], z, [qt, T, P], [];
systems = [sdata_T, sdata_P]
)
structural_simplify(model_two_data) # failsERROR: BoundsError: attempt to access 4-element Vector{Union{ModelingToolkit.BipartiteGraphs.Unassigned, ModelingToolkit.StructuralTransformations.SelectedState, Int64}} at index [5]
Stacktrace:
[1] getindex
@ ./essentials.jl:13 [inlined]
[2] getindex
@ ~/.julia/packages/ModelingToolkit/32WwH/src/bipartite_graph.jl:60 [inlined]
[3] tearing_reassemble(state::TearingState{…}, var_eq_matching::ModelingToolkit.BipartiteGraphs.Matching{…}; simplify::Bool, mm::ModelingToolkit.SparseMatrixCLIL{…})
@ ModelingToolkit.StructuralTransformations ~/.julia/packages/ModelingToolkit/32WwH/src/structural_transformation/symbolics_tearing.jl:433
[4] tearing_reassemble
@ ~/.julia/packages/ModelingToolkit/32WwH/src/structural_transformation/symbolics_tearing.jl:220 [inlined]
[5] #dummy_derivative#132
@ ~/.julia/packages/ModelingToolkit/32WwH/src/structural_transformation/symbolics_tearing.jl:637 [inlined]
[6] _structural_simplify!(state::TearingState{…}, io::Nothing; simplify::Bool, check_consistency::Bool, fully_determined::Bool, kwargs::@Kwargs{})
@ ModelingToolkit ~/.julia/packages/ModelingToolkit/32WwH/src/systems/systemstructure.jl:0
[7] _structural_simplify!
@ ~/.julia/packages/ModelingToolkit/32WwH/src/systems/systemstructure.jl:617 [inlined]
[8] structural_simplify!(state::TearingState{…}, io::Nothing; simplify::Bool, check_consistency::Bool, fully_determined::Bool, kwargs::@Kwargs{})
@ ModelingToolkit ~/.julia/packages/ModelingToolkit/32WwH/src/systems/systemstructure.jl:580
[9] __structural_simplify(sys::ODESystem, io::Nothing; simplify::Bool, kwargs::@Kwargs{})
@ ModelingToolkit ~/.julia/packages/ModelingToolkit/32WwH/src/systems/systems.jl:0
[10] __structural_simplify
@ ~/.julia/packages/ModelingToolkit/32WwH/src/systems/systems.jl:38 [inlined]
[11] structural_simplify(sys::ODESystem, io::Nothing; simplify::Bool, split::Bool, kwargs::@Kwargs{})
@ ModelingToolkit ~/.julia/packages/ModelingToolkit/32WwH/src/systems/systems.jl:22
[12] structural_simplify
@ ~/.julia/packages/ModelingToolkit/32WwH/src/systems/systems.jl:19 [inlined]
[13] structural_simplify(sys::ODESystem)
@ ModelingToolkit ~/.julia/packages/ModelingToolkit/32WwH/src/systems/systems.jl:19
[14] top-level scope
@ ~/projects/test_mtk/mtk_test.jl:35
Some type information was truncated. Use `show(err)` to see complete types.Found a workaround: RealOutput also uses t, so replacing that with an equivalent RealOutputZ worked. Leaving this closed but keeping these comments up in case anyone else needs this hack.
Describe the bug 🐞
I'm trying to create a system that uses
SampledDatablocks, so I can run many ODE solves in parallel with slightly different input data (like a parameter sweep but vectorial). When I try to do this as is done in theSampledDatadocs with more than oneSampledDatablock, I get an error instructural_simplify.Expected behavior
I expected
structural_simplifyto run correctly and return a model with fewer equations. This is what happens with oneSampledDatablock.Minimal Reproducible Example 👇
Error & Stacktrace ⚠️
Environment (please complete the following information):
using Pkg; Pkg.status()using Pkg; Pkg.status(; mode = PKGMODE_MANIFEST)versioninfo()Additional context
I tried rebuilding this example based on the SampledData docs, but those don't seem to run correctly now; I'll open a separate issue for this on the ModelingToolkitStandardLibrary github. This seems like an MTK issue more than a standard library one because it relates to the internal functioning of
structural_simplify.