pybamm-team / PyBaMM

Fast and flexible physics-based battery models in Python
https://www.pybamm.org/
BSD 3-Clause "New" or "Revised" License
1.07k stars 528 forks source link

Improvements to `QuickPlot` #871

Closed valentinsulzer closed 4 years ago

valentinsulzer commented 4 years ago

QuickPlot could do with a few updates. Feel free to add more to this list:

Scottmar93 commented 4 years ago

My thoughts are:

  1. Agree
  2. Agree
  3. Yes we really need this.
  4. If spatial scale not found then raise error
  5. Yes defaults are good for pybamm.QuickPlot
  6. Okay
  7. Okay
  8. Sure. Maybe just make quickplot a method of simulation that uses the default variables.

Some additional ideas that are non essential but might be nice:

Another thought though this really is just extra. It might be nice to have helper functions in simulation:

rtimms commented 4 years ago

My thoughts:

valentinsulzer commented 4 years ago

All sound like good points. re: linestyles, the current functionality is that you can plot several variables on the same plot and they will be differentiated by the linestyles, which is why the different solutions are only differentiated by colour. For example

pybamm.QuickPlot(
    solution,
    output_variables=[
        "Negative particle surface concentration",
        "Positive particle surface concentration",
        ["Electrolyte current density", "Electrode current density"],
        "Terminal voltage [V]",
    ],
)

gives example

rtimms commented 4 years ago

ah nice! I didn't know you could nest lists in that way to use in QuickPlot

Scottmar93 commented 4 years ago

When we call ipywidgets within simulation for slider plots in jupyter notebook, I think we should also pass the argument continuous_update=False as this seems to give more responsive behaviour :)

valentinsulzer commented 4 years ago

Are you thinking to pass this in from the notebook or detect that it is being run from a notebook?

Scottmar93 commented 4 years ago

Was thinking to just change line 536 of simulation to:

widgets.interact(
                plot.plot,
                t=widgets.FloatSlider(min=0, max=plot.max_t, step=0.05, value=0),
                continuous_update=False
            )
Scottmar93 commented 4 years ago

Regarding linestyles, that is really neat and I didn't know you could do that either. I still think it would be nice to do models by linestyle though (if multiple variables are not plotted on the same figure) since I would use this more frequently.

rtimms commented 4 years ago

can we just move isnotebook to QuickPlot so we can use it directly in notebooks without having to do the slider widget manually?

Scottmar93 commented 4 years ago

One additional thing that would be nice, is if QuickPlot can accept solution objects and simulation objects. If a simulation object is passed, it just does simulation.solution automatically. I am finding myself writing [sim1.solution, sim2.solution, etc] or having to do [sim.solution for sim in [sim1, sim2, sim3]] quite a bit, which is just messy

valentinsulzer commented 4 years ago