Skip to content

Conversation

@shashi
Copy link
Member

@shashi shashi commented Feb 16, 2022

@codecov-commenter
Copy link

codecov-commenter commented Mar 10, 2022

Codecov Report

Merging #530 (23880b9) into master (ffa417d) will increase coverage by 69.59%.
The diff coverage is 90.90%.

@@             Coverage Diff             @@
##           master     #530       +/-   ##
===========================================
+ Coverage    9.02%   78.62%   +69.59%     
===========================================
  Files          23       23               
  Lines        2272     2334       +62     
===========================================
+ Hits          205     1835     +1630     
+ Misses       2067      499     -1568     
Impacted Files Coverage Δ
src/diff.jl 79.92% <90.90%> (+79.52%) ⬆️
src/init.jl 100.00% <0.00%> (ø)
src/register.jl 71.42% <0.00%> (+6.72%) ⬆️
src/wrapper-types.jl 96.15% <0.00%> (+15.33%) ⬆️
src/extra_functions.jl 16.66% <0.00%> (+16.66%) ⬆️
src/linearity.jl 62.36% <0.00%> (+37.63%) ⬆️
src/domains.jl 44.44% <0.00%> (+44.44%) ⬆️
src/variable.jl 78.44% <0.00%> (+54.36%) ⬆️
src/latexify_recipes.jl 54.78% <0.00%> (+54.78%) ⬆️
src/Symbolics.jl 64.28% <0.00%> (+57.14%) ⬆️
... and 12 more

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

is_scalar_indexed(ex) = istree(ex) && operation(x) == getindex && !(symtype(x) <: AbstractArray)
if is_scalar_indexed(x) && is_scalar_indexed(expr) &&
isequal(first(arguments(x)), first(arguments(expr)))
return isequal(arguments(x), arguments(expr))
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this might be the problem for the DataDriven failure.

using ModelingToolkit
using Symbolics
@variables u[1:2] y[1:1] t
u = collect(u)
y = collect(y)
Symbolics.jacobian([u;u[1]^2; y], u)

fails while

using ModelingToolkit
using Symbolics
@variables u[1:2] y[1:1] t
u = collect(u)
y = collect(y)
Symbolics.jacobian([u;u[1]^2], u)

is valid. The problem is that I take the Jacobian w.r.t. to the state variables only to derive a variable transform. If the system has control variables, there are mixed terms. Could this be relaxed to
all([isin(xi, arguments(expr) for xi in arguments(x)]) ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah okay, that can be fixed.

Copy link
Member Author

Choose a reason for hiding this comment

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

Try out the latest change.

@shashi shashi merged commit b077a79 into master Mar 14, 2022
@shashi shashi deleted the s/fail-array-diff branch March 14, 2022 20:04
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.

4 participants