handle connections in change_independent_variable #3625
Conversation
|
I'm not super familiar with the Maybe @ChrisRackauckas wants to have a quick look? |
All connections are linear combinations so it should be fine to handle them the simple way. |
| @@ -158,7 +158,7 @@ function change_independent_variable( | |||
| function transform(ex::T) where {T} | |||
| # 1) Replace the argument of every function; e.g. f(t) -> f(u(t)) | |||
| for var in vars(ex; op = Nothing) # loop over all variables in expression (op = Nothing prevents interpreting "D(f(t))" as one big variable) | |||
| is_function_of_iv1 = iscall(var) && isequal(only(arguments(var)), iv1) # of the form f(t)? | |||
| is_function_of_iv1 = iscall(var) && isequal(first(arguments(var)), iv1) # of the form f(t)? | |||
There was a problem hiding this comment.
why is this needed? It should be only or error?
There was a problem hiding this comment.
reverted back to only, @hersle PR handles this correctly.
|
This looks reasonable, though I'd be scared of changing that |
@ChrisRackauckas This was an artifact of me trying to allow symbolic arrays to be similarly handled. It was failing previously as the only was hitting cases of e.g. |
|
Oh I see. Hmm, okay that's fine. |
|
Also, I tried implementing this first by simply transforming the systems in each connection and then rebuilding the connection (without the use of the Is this something you would expect? |
|
Retriggering tests post scimlbase hotfix |
That is not something I'd expect. I'm not sure why the display would care? You may not have reconstructed the |
Gotcha. If we are okay with the current approach (which saves us from re-transforming a bunch of systems) I will just use this. |
Checklist
contributor guidelines, in particular the SciML Style Guide and
COLPRAC.
Additional context
Expand
change_independent_variableto handle systems with connections. This was chosen over forcingexpand_connectionsprior to calling, asexpand_connectionsforces flatness and that is undesirable in some cases.