Feature/temperature effects

[lightningclaw001]

-added in non constant temperature diffusion/thermal diffusion effects in electrolyte only
-added in heat generation from reactions, ohmic heat, and entropy

[lightningclaw001]

@d-cogswell can we take a look at these changes some time too? I'm going to start implementing the heat generation part after this is merged in.

[d-cogswell]

I added some code comments.

[d-cogswell]

Why use a central difference? What happens at the electrode/separator interface if the computational cell size changes?

[d-cogswell]

Transport properties are usually the weighted harmonic mean of the neighboring cells?

[d-cogswell]

'dxd1' is the distance between neighboring cell centers, but what is 'dxd2'?

[d-cogswell]

Is it possible to write this with fewer ifs and loops?

[lightningclaw001]

I attempted to do this previously but I think it's pretty difficult because we need to consider cases where there is no separator, where the separator information should be set to 0 for every point on the separator, or where the electrode for the anode doesn't exist, which actually means that we should have no data at the anode.

[lightningclaw001]

@d-cogswell @loostrum , just wondering, does the Fuller test pass on your computers? on my local environment it passes.

[loostrum]

It's passing for me as well. The ValueError is not very insightful, it just means the error is in one of these lines:

varDataNew = newData[varKey][...]
varDataRef = refData[varKey][...]
diffMat = np.abs(varDataNew - varDataRef)

Which is just selecting the right data so it's strange that it's failing there. I'll add a line to print more info when things fail.

[loostrum]

Apparently there is a shape mismatch, Fail from ValueError: operands could not be broadcast together with shapes (1,19) (1,20)
Although strangely the shapes themselves are different on Python 3.8 vs other versions, and on Python 3.10 the test is now passing. It's a bit concerning that even the shape of the reference output depends on the Python version. Hopefully this helps pinning down the issue.

[lightningclaw001]

Apparently there is a shape mismatch, Fail from ValueError: operands could not be broadcast together with shapes (1,19) (1,20) Although strangely the shapes themselves are different on Python 3.8 vs other versions, and on Python 3.10 the test is now passing. It's a bit concerning that even the shape of the reference output depends on the Python version. Hopefully this helps pinning down the issue.

@loostrum I've had this problem happen before, but sometimes the solutions just print an extra time point. It was quite odd and I never pinpointed down the reason it happened. Do you think it is related to the tolerances?

[d-cogswell]

I'm hunting for the commit that first broke that test. Maybe that will give us some clues.

[d-cogswell]

The failing Fuller test first occurs in b4feee4. Maybe we should revert to the previous commit 14de592?

[lightningclaw001]

The failing Fuller test first occurs in b4feee4. Maybe we should revert to the previous commit 14de592?

This would work... I think the only change is from removing the ghost points and reducing the number of equations. From what I recall, the solution plots were exactly the same, with an extra time point. Maybe the ghost point method just adds another point or the tolerance is just really close? Let me try and revert this commit to see if it passes (though to test if it passes on Github I'm going to have to push it)

[lightningclaw001]

@loostrum I think I broke something in the DAEtools tests in python 3.9... 🙃

So I reran it and then it was fine... I have no idea what is going on. Race condition?

[lightningclaw001]

@d-cogswell all tests passing now! yay!

[d-cogswell]

Hi @Ombrini , we're using this branch for implementing non-constant temperature. Please keep your own work in a separate branch or fork. Thanks!