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

Names of unknowns have a (t) appendix but DataFrame column names not #798

Closed
david-hofmann opened this issue Sep 25, 2024 · 4 comments · Fixed by #813
Closed

Names of unknowns have a (t) appendix but DataFrame column names not #798

david-hofmann opened this issue Sep 25, 2024 · 4 comments · Fixed by #813
Assignees

Comments

@david-hofmann
Copy link

A nice to have: consistency between unknown names and DataFrame column names

The names of unknowns/variables of an ODESystem come with a (t) appendix. However, when transforming the solution to a DataFrame the (t) is gone. This way one cannot directly use the name of a variable as an index.
It would be helpful to make naming consistent.

Minimal Reproducible Example 👇

using ModelingToolkit
using DataFrames
using OrdinaryDiffEq

const t = ModelingToolkit.t_nounits
const D = ModelingToolkit.D_nounits

@variables x(t)=1.0
eqn = [D(x) ~ 0.0]

@named sys = ODESystem(eqn,t,[x],[],tspan=(0, 10.0))
sys = structural_simplify(sys)
prob = ODEProblem(sys)
sol = solve(prob)

dfsys = DataFrame(sol)

String(Symbol(unknowns(sys)[1]))
names(dfsys)[2]

Package Versions

Status `~/.julia/environments/v1.10/Project.toml`
  [13f3f980] CairoMakie v0.12.11
⌃ [a93c6f00] DataFrames v1.6.1
⌃ [0c46a032] DifferentialEquations v7.13.0
  [86223c79] Graphs v1.11.2
  [23992714] MAT v0.10.7
⌃ [961ee093] ModelingToolkit v9.40.0
  [769b91e5] Neuroblox v0.4.2 `../../../Projects/neuroblox/codes/Neuroblox.jl`
  [bac558e1] OrderedCollections v1.6.3
⌃ [1dea7af3] OrdinaryDiffEq v6.87.0
⌃ [91a5bcdd] Plots v1.40.1
⌃ [c3e4b0f8] Pluto v0.19.45

Thank you!

@david-hofmann david-hofmann changed the title Names of unknowns have a (t) appendix but DataFrames without Names of unknowns have a (t) appendix but DataFrame column names not Sep 25, 2024
@ChrisRackauckas
Copy link
Member

@AayushSabharwal I don't think this change is intended right?

@david-hofmann workflow wise, what's the reason for preferring one over the other?

@david-hofmann
Copy link
Author

From my (code's) perspective both are equivalent. I can adapt the code either way, it should just be consistent.

@AayushSabharwal
Copy link
Member

AayushSabharwal commented Sep 27, 2024

This hasn't changed in like 9 months since SII 0.3 came out 😅 but it is an inconsistency, since if you convert a DiffEqArray to a DataFrame it'll add the (t) but solutions won't because we have the Tables.jl interface implemented twice for some reason. I'll remove one and/or fix the other depending on which version we want to keep.

@david-hofmann
Copy link
Author

I'd appreciate to have everything with the (t) appendix if there is no reason not to have that. No particular reason, just a preference.

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 a pull request may close this issue.

3 participants