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

Simplify printing #610

Closed
wants to merge 2 commits into from
Closed

Simplify printing #610

wants to merge 2 commits into from

Conversation

ChrisRackauckas
Copy link
Member

With the changes downstream, this makes var"mysymbol.x" just print as mysymbol.x, which makes all of the use cases look so much nicer and gets rid of unicode.

julia> rc_model
Model rc_model with 1 equations
Unknowns (1):
  capacitor.v
Parameters (3):
  resistor.R [defaults to 2.0]
  capacitor.C [defaults to 1.0]

Incidence matrix:1×2 SparseArrays.SparseMatrixCSC{Num, Int64} with 2 stored entries:
 ×  ×

julia> equations(rc_model)
1-element Vector{Equation}:
 Differential(t)(capacitor.v) ~ capacitor.i / capacitor.C

julia> unknowns(rc_model)
1-element Vector{SymbolicUtils.BasicSymbolic{Real}}:
 capacitor.v

julia> parameters(rc_model)
3-element Vector{SymbolicUtils.BasicSymbolic{Real}}:
 resistor.R
 capacitor.C
 source.V

julia> observed(rc_model)
19-element Vector{Equation}:
 source.v ~ source.V
 capacitor.p.v ~ capacitor.v
 capacitor.n.v ~ -0.0
 ground.g.v ~ 0.0
 
 source.p.i ~ source.i
 capacitor.p.i ~ -capacitor.n.i
 source.n.i ~ -source.p.i

With the changes downstream, this makes `var"mysymbol.x"` just print as `mysymbol.x`, which makes all of the use cases look so much nicer and gets rid of unicode.

```julia
julia> rc_model
Model rc_model with 1 equations
Unknowns (1):
  capacitor.v
Parameters (3):
  resistor.R [defaults to 2.0]
  capacitor.C [defaults to 1.0]
⋮
Incidence matrix:1×2 SparseArrays.SparseMatrixCSC{Num, Int64} with 2 stored entries:
 ×  ×

julia> equations(rc_model)
1-element Vector{Equation}:
 Differential(t)(capacitor.v) ~ capacitor.i / capacitor.C

julia> unknowns(rc_model)
1-element Vector{SymbolicUtils.BasicSymbolic{Real}}:
 capacitor.v

julia> parameters(rc_model)
3-element Vector{SymbolicUtils.BasicSymbolic{Real}}:
 resistor.R
 capacitor.C
 source.V

julia> observed(rc_model)
19-element Vector{Equation}:
 source.v ~ source.V
 capacitor.p.v ~ capacitor.v
 capacitor.n.v ~ -0.0
 ground.g.v ~ 0.0
 ⋮
 source.p.i ~ source.i
 capacitor.p.i ~ -capacitor.n.i
 source.n.i ~ -source.p.i
```
Copy link
Contributor

github-actions bot commented Jun 12, 2024

Benchmark Results

master 7515cfa... master/7515cfa7951353...
overhead/acrule/a+2 0.767 ± 0.023 μs 0.733 ± 0.023 μs 1.05
overhead/acrule/a+2+b 0.743 ± 0.023 μs 0.71 ± 0.023 μs 1.05
overhead/acrule/a+b 0.265 ± 0.0082 μs 0.256 ± 0.0075 μs 1.04
overhead/acrule/noop:Int 25.9 ± 0.05 ns 25.9 ± 0.92 ns 1
overhead/acrule/noop:Sym 0.0359 ± 0.0058 μs 0.0372 ± 0.0055 μs 0.965
overhead/rule/noop:Int 0.0463 ± 0.0022 μs 0.0446 ± 0.00056 μs 1.04
overhead/rule/noop:Sym 0.057 ± 0.0032 μs 0.0575 ± 0.0035 μs 0.992
overhead/rule/noop:Term 0.0566 ± 0.003 μs 0.0564 ± 0.0039 μs 1
overhead/ruleset/noop:Int 0.134 ± 0.0034 μs 0.128 ± 0.0028 μs 1.05
overhead/ruleset/noop:Sym 0.165 ± 0.0073 μs 0.15 ± 0.0069 μs 1.1
overhead/ruleset/noop:Term 3.82 ± 0.21 μs 3.85 ± 0.19 μs 0.992
overhead/simplify/noop:Int 0.152 ± 0.003 μs 0.14 ± 0.002 μs 1.09
overhead/simplify/noop:Sym 0.164 ± 0.0077 μs 0.161 ± 0.0045 μs 1.01
overhead/simplify/noop:Term 0.0393 ± 0.0025 ms 0.0402 ± 0.0028 ms 0.979
overhead/simplify/randterm (+, *):serial 0.126 ± 0.002 s 0.13 ± 0.0022 s 0.97
overhead/simplify/randterm (+, *):thread 0.0762 ± 0.026 s 0.0773 ± 0.027 s 0.985
overhead/simplify/randterm (/, *):serial 0.234 ± 0.0091 ms 0.235 ± 0.011 ms 0.993
overhead/simplify/randterm (/, *):thread 0.266 ± 0.011 ms 0.27 ± 0.012 ms 0.987
overhead/substitute/a 0.0582 ± 0.002 ms 0.0585 ± 0.002 ms 0.995
overhead/substitute/a,b 0.0515 ± 0.0021 ms 0.0525 ± 0.0023 ms 0.982
overhead/substitute/a,b,c 17.3 ± 0.87 μs 18 ± 0.97 μs 0.966
polyform/easy_iszero 0.0338 ± 0.0023 ms 0.0352 ± 0.0025 ms 0.962
polyform/isone 2.79 ± 0.01 ns 2.79 ± 0.01 ns 1
polyform/iszero 1.84 ± 0.047 ms 1.88 ± 0.065 ms 0.978
polyform/simplify_fractions 2.58 ± 0.071 ms 2.63 ± 0.091 ms 0.979
time_to_load 4.87 ± 0.017 s 4.89 ± 0.045 s 0.995

Benchmark Plots

A plot of the benchmark results have been uploaded as an artifact to the workflow run for this PR.
Go to "Actions"->"Benchmark a pull request"->[the most recent run]->"Artifacts" (at the bottom).

@ChrisRackauckas ChrisRackauckas requested a review from YingboMa June 12, 2024 14:06
@YingboMa
Copy link
Member

This will break model text serialization.

Copy link
Member

@YingboMa YingboMa left a comment

Choose a reason for hiding this comment

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

Printing with quotation is deliberate to support text-based model serialization.

@ChrisRackauckas
Copy link
Member Author

This would make copy-paste work, which would mean directly writing to a file would reconstruct the same expressions. I presume you must mean something different by text serialization?

@YingboMa
Copy link
Member

@variables t x.y isn't valid syntax.

@YingboMa
Copy link
Member

I meant something like

julia> rc_model
Model rc_model with 14 (22) equations
Unknowns (22):
  resistor₊v(t) [defaults to 0.0]
  resistor₊i(t) [defaults to 0.0]
  resistor₊p₊v(t)
  resistor₊p₊i(t)
  resistor₊n₊v(t)
  resistor₊n₊i(t)
  capacitor₊v(t) [defaults to 0.0]
  capacitor₊i(t) [defaults to 0.0]
  capacitor₊p₊v(t)
  capacitor₊p₊i(t)
  capacitor₊n₊v(t)
  capacitor₊n₊i(t)
  input₊output₊u(t): Inner variable in RealOutput output

Parameters (7):
  resistor₊R [defaults to 1.0]: Resistance
  capacitor₊C [defaults to 1.0]: Capacitance
  input₊offset [defaults to 1]
  input₊start_time [defaults to 1]
  input₊amplitude [defaults to 1]
  input₊frequency [defaults to 1]
  input₊phase [defaults to 1]

julia> toexpr(rc_model)
:(let
      begin
          var"##iv#14121" = (#= /Users/schemescheme/src/julia/ModelingToolkit/src/systems/abstractsystem.jl:1250 =# @variables(t))[1]
          var"##sts#14122" = (collect)(@variables(resistor₊v(t), resistor₊i(t), resistor₊p₊v(t), resistor₊p₊i(t), resistor₊n₊v(t), resistor₊n₊i(t), capacitor₊v(t), capacitor₊i(t), capacitor₊p₊v(t), capacitor₊p₊i(t), capacitor₊n₊v(t), capacitor₊n₊i(t), input₊output₊u(t), source₊v(t), source₊i(t), source₊p₊v(t), source₊p₊i(t), source₊n₊v(t), source₊n₊i(t), source₊V₊u(t), ground₊g₊v(t), ground₊g₊i(t)))
          var"##ps#14123" = (collect)(@parameters(resistor₊R, capacitor₊C, input₊offset, input₊start_time, input₊amplitude, input₊frequency, input₊phase))
          var"##eqs#14125" = [(Symbolics.connect)(input.output, source.V); (Symbolics.connect)(source.p, resistor.p); (Symbolics.connect)(resistor.n, capacitor.p); (Symbolics.connect)(capacitor.n, source.n, ground.g); (~)(resistor₊v, (+)((*)(-1, resistor₊n₊v), resistor₊p₊v)); (~)(0, (+)(resistor₊p₊i, resistor₊n₊i)); (~)(resistor₊i, resistor₊p₊i); (~)(resistor₊v, (*)(resistor₊R, resistor₊i)); (~)(capacitor₊v, (+)(capacitor₊p₊v, (*)(-1, capacitor₊n₊v))); (~)(0, (+)(capacitor₊p₊i, capacitor₊n₊i)); (~)(capacitor₊i, capacitor₊p₊i); (~)((Differential(t))(capacitor₊v), (/)(capacitor₊i, capacitor₊C)); (~)(input₊output₊u, (+)(input₊offset, (ifelse)((<)(t, input₊start_time), 0, (*)(input₊amplitude, (sin)((+)(input₊phase, (*)(6.283185307179586, input₊frequency, (+)((*)(-1, input₊start_time), t)))))))); (~)(source₊v, (+)((*)(-1, source₊n₊v), source₊p₊v)); (~)(0, (+)(source₊p₊i, source₊n₊i)); (~)(source₊i, source₊p₊i); (~)(source₊v, source₊V₊u); (~)(ground₊g₊v, 0)]
          var"##defs#14126" = (Dict)((Pair)(resistor₊v, 0.0), (Pair)(input₊frequency, 1), (Pair)(input₊offset, 1), (Pair)(capacitor₊i, 0.0), (Pair)(resistor₊i, 0.0), (Pair)(input₊start_time, 1), (Pair)(resistor₊R, 1.0), (Pair)(source₊v, 0.0), (Pair)(input₊phase, 1), (Pair)(capacitor₊v, 0.0), (Pair)(input₊amplitude, 1), (Pair)(source₊i, 0.0), (Pair)(capacitor₊C, 1.0))
          var"##eqs#14127" = [;]
          var"##iv#14128" = (#= /Users/schemescheme/src/julia/ModelingToolkit/src/systems/abstractsystem.jl:1276 =# @variables(t))[1]
          (ODESystem)(var"##eqs#14125", var"##iv#14128", var"##sts#14122", var"##ps#14123"; defaults = var"##defs#14126", observed = var"##eqs#14127", name = :rc_model, checks = false)
      end
  end)

@ChrisRackauckas
Copy link
Member Author

I see, so we should specialize the display of systems and equations specifically?

@YingboMa
Copy link
Member

Yeah, I think we should decouple display of models further. We can add a PrettyPrint struct and overload show for PrettyPrint(model, :eqs) etc.

@ChrisRackauckas ChrisRackauckas deleted the print_simplify branch June 12, 2024 16:47
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.

2 participants