JuliaSymbolics / Symbolics.jl

Symbolic programming for the next generation of numerical software
https://docs.sciml.ai/Symbolics/stable/
Other
1.36k stars 151 forks source link

`get_variables` order #1182

Closed schillic closed 1 month ago

schillic commented 3 months ago

A recent change about sorting in SymbolicUtils changed the behavior of get_variables here. Is there a contract about the order in which the variables are returned? The documentation does not say anything about that, but I have the impression that they were sorted before, so for me this was a breaking change. More confusingly, the function now returns a different result when receiving a parsed expression object or an expression for parsing:

Old behavior
pkg> add SymbolicUtils@2.0

julia> using Symbolics

julia> vars = @variables x y
2-element Vector{Num}:
 x
 y

julia> Symbolics.get_variables(x - y)
2-element Vector{Any}:
 x
 y

julia> expr = x - y
x - y

julia> Symbolics.get_variables(expr)
2-element Vector{Any}:
 x
 y
New behavior
pkg> add SymbolicUtils@2.1, Symbolics@5.31

julia> using Symbolics

julia> vars = @variables x y
2-element Vector{Num}:
 x
 y

julia> Symbolics.get_variables(x - y)  # order swapped here
2-element Vector{Any}:
 y
 x

julia> expr = x - y
x - y

julia> Symbolics.get_variables(expr)  # order not swapped here
2-element Vector{Any}:
 x
 y

Could you please confirm that this is expected behavior, and clarify how I can obtain a consistent order of the variables?

ChrisRackauckas commented 3 months ago

Yes, there was no guarantee on the sorting order, and a recent change removes a previously enforced sorting which greatly improves performance. @bowenszhu is there a sorted version of this function?

bowenszhu commented 3 months ago

Symbolics.get_variables doesn't offer a built-in way to obtain a sorted list of variables (as of Symbolics v5.32.0).

@ChrisRackauckas Currently, we have arguments and sorted_arguments. To enhance flexibility, how about we remove sorted_arguments, modify arguments(x) to accept a sort keyword argument arguments(x; sort = false) and add a kwargs... argument to relevant function signatures? This allows users to explicitly control sorting behavior.

ChrisRackauckas commented 3 months ago

I think that was being discussed with @0x0f0f0f ? I think that's a good idea.

0x0f0f0f commented 3 months ago

@bowenszhu Can't users already control this? We discussed (It took us 3 years) to avoid keyword arguments because they make implementing TermInterface.jl unstraightforward. It would be a bit of a PITA to change TermInterface again and go through another major release cycle. This is expected and I've warned about this in https://github.com/JuliaSymbolics/SymbolicUtils.jl/pull/609

What about we just change get_variables to use sorted_arguments? Was there any guarantee in what order should get_variables have returned the variables? In order of appearance? Lexicographically ordered?

I guess the latter, with <\_e would be ok

schillic commented 1 month ago

A change in get_variables, either by default or via an additional argument, would be just fine. Or really any other way to get variables sorted (lexicographically or order of definition should be sufficient).

I am a bit surprised that nobody else seems to be needing this feature. We use Symbolics to parse expressions like 2x + 3y == 5 and to extract the a and b from the pattern a·z == b. Previously I got the expected a = [2, 3], but with the new version, this now results in a = [3, 2], which makes it not usable anymore. Maybe using Symbolics for that is overkill, but it used to work nicely.

ChrisRackauckas commented 1 month ago

Can't we just do sorted_get_variables which just uses sorted_arguments and this is done?