Bug when mixing symmetric and asymmetric conductance in different domains

[mkaguer]

Please refer to this line here

I ran into this problem when working on my lithium ion battery model. In the electrolyte phase I have diffusive-migrative conductance that is asymmetric meaning that the conductance is a Nt by 2 array. However, in the active material phase, I have pure diffusive conductance that is a Nt by 1 array. The code runs into a problem when it tries to merge these models from each domain. The bug occurs at the line I linked to above. To get around this issue, I wrote my own diffusive conductance model in the active material phase that writes it as an Nt by 2 array (where the conductance is the same for the first and second columns). I know this is how OpenPNM used to return conductances. Maybe we should revert back to this? Or be able to merge symmetric and asymmetric conductances together.

[ma-sadeghi]

I'd say let's always use Nt by 2. One downside is memory footprint, but we're storing lots of other properties (like T, p, etc.), so it's not really noticeable. Any other downsides? @jgostick @mkaguer

[jgostick]

I can't think of any reason why not. Just be careful with the ordering of the elements, like C vs F. I recently rediscovered that you can't just flatten our conns array and use it an an index because the two columns interlace rather than stack. I mention this because Mehrez has an explicit comment about this where he builds the Nt-by-2 conductances for advection-diffusion.

[mkaguer]

Should I go ahead and fix this then? I will make all conductances Nt-by-2.

[ma-sadeghi]

Yes, please make PR

[ma-sadeghi]

While you're at it, you could also add a test that goes through all our conductance models and ensures the shape is correct. Don't hardcore it, you can programmatically do this so it'll catch future conductance models as well