pybamm-team / PyBaMM

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

Colorbar on 2d data shows wrong values #1145

Open TomTranter opened 4 years ago

TomTranter commented 4 years ago

Describe the bug I was running a simulation where temperature in a 1+1D model wasn't changing much (less than 1 degree) but the colorbar made it look like it varied by 10s of degrees because it is adjusted up and down by 4% of the absolute value (298) after plotting the mesh when variable_limits="tight". First of all I'm not sure it should be adjusting it at all after plotting and refreshing. Secondly 4% of the data range would make more sense.

To Reproduce Steps to reproduce the behaviour:

import pybamm

param = pybamm.ParameterValues(chemistry=pybamm.parameter_sets.Marquis2019)
model_1D_lumped = pybamm.lithium_ion.SPMe({"dimensionality": 1,
                                           "current collector": "potential pair",
                                           "thermal": "x-lumped"})
sim_1D_lumped = pybamm.Simulation(model_1D_lumped, parameter_values=param)
sim_1D_lumped.solve([0, 36])
temp = sim_1D_lumped.solution["Cell temperature [K]"].entries
t_min = temp.min()
t_max = temp.max()
variable_limits = {"X-averaged cell temperature [K]": (t_min, t_max),
                   "Cell temperature [K]": (t_min, t_max),
                   "Volume-averaged cell temperature [K]": (t_min, t_max)}
print(t_min, t_max)
sim_1D_lumped.plot(["X-averaged cell temperature [K]",
                    "Cell temperature [K]",
                    "Volume-averaged cell temperature [K]"],
                    variable_limits="tight",
                    )

Expected behaviour A true representation of the temperature range that update dynamically for each time step

Screenshots If applicable, add screenshots to help explain your problem. image

TomTranter commented 4 years ago

Using the actual variable limits from the dictionary instead gives the best results. image

Not specifying the limits again makes the range too wide but at least shows the right colour and scale image

I am happy to make the changes and adjust the limits based on the data range but also make sure what is shown on the colorbar is the same as the plot - 2 conflated issues here really

valentinsulzer commented 4 years ago

So, just to understand, are you suggesting that the colorbar should be fixed to (min of variable, max of variable) even when variable_limits is "tight"? Which would be the same behaviour (for 2d data) as having variable_limits="fixed"?

valentinsulzer commented 4 years ago

i.e. does the following give you exactly what you want?

variable_limits = {"X-averaged cell temperature [K]": "tight",
                   "Cell temperature [K]": "fixed",
                   "Volume-averaged cell temperature [K]": "tight"}

If so, we could add an option which sets 1D variables to tight and 2D variables to fixed

TomTranter commented 4 years ago

I think there is a genuine bug. In the first screen shot you see the variation in temperature and corresponding colorbar and they have been scaled differently. ax_min and ax_max increase the range on the colorbar only so my small temperature difference looked as though it was massive.

valentinsulzer commented 4 years ago

Ah yeah, I see

valentinsulzer commented 3 years ago

@TomTranter this might be a good first issue for someone if you don't have time to do and can explain exactly what is needed

TomTranter commented 3 years ago

Yep, I’d forgotten about this until the other day

On Thu, 4 Feb 2021 at 21:52, Valentin Sulzer notifications@github.com wrote:

@TomTranter https://github.com/TomTranter this might be a good first issue for someone if you don't have time to do and can explain exactly what is needed

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/pybamm-team/PyBaMM/issues/1145#issuecomment-773627349, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABV5YROC7T4ZPBUE2QM2KADS5MJKBANCNFSM4RFWP3YQ .

Saransh-cpp commented 3 years ago

Can you please elaborate on this more? I tried running the code mentioned above with different values and yes, only the plots with variable_limits not tight gave the right color. From the discussion above I understand that the ideal output should be a smaller range, given that the temperature is changing by a small amount in the experiment and a relevant color.

1

variable_limits = {"X-averaged cell temperature [K]": "tight",
                   "Cell temperature [K]": "fixed",
                   "Volume-averaged cell temperature [K]": "tight"}
print(t_min, t_max)
sim_1D_lumped.plot(["X-averaged cell temperature [K]",
                    "Cell temperature [K]",
                    "Volume-averaged cell temperature [K]"],
                    variable_limits="tight",
                    )

image

2

variable_limits = {"X-averaged cell temperature [K]": "tight",
                   "Cell temperature [K]": "fixed",
                   "Volume-averaged cell temperature [K]": "tight"}
print(t_min, t_max)
sim_1D_lumped.plot(["X-averaged cell temperature [K]",
                    "Cell temperature [K]",
                    "Volume-averaged cell temperature [K]"],
                    # variable_limits="tight",
                    )

image

3

variable_limits = {"X-averaged cell temperature [K]": (t_min, t_max),
                   "Cell temperature [K]": (t_min, t_max),
                   "Volume-averaged cell temperature [K]": (t_min, t_max)}
print(t_min, t_max)
sim_1D_lumped.plot(["X-averaged cell temperature [K]",
                    "Cell temperature [K]",
                    "Volume-averaged cell temperature [K]"],
                    variable_limits="tight",
                    )

image

4

variable_limits = {"X-averaged cell temperature [K]": (t_min, t_max),
                   "Cell temperature [K]": (t_min, t_max),
                   "Volume-averaged cell temperature [K]": (t_min, t_max)}
print(t_min, t_max)
sim_1D_lumped.plot(["X-averaged cell temperature [K]",
                    "Cell temperature [K]",
                    "Volume-averaged cell temperature [K]"],
                    # variable_limits="tight",
                    )

image

The code also throws a warning saying

UserWarning: 1+1D Thermal models are only valid if both tabs are placed at the top of the cell.
valentinsulzer commented 3 years ago

@TomTranter I wasn't able to reproduce the problem with different scalings. Do you still see it or have we accidentally fixed it?

rtimms commented 1 year ago

I think this is fixed. @TomTranter can you test and confirm?