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

Diffusion: tests, fixed coefficients in the solver, & tutorial #2226

Open
wants to merge 90 commits into
base: master
Choose a base branch
from

Conversation

thorstenhater
Copy link
Contributor

@thorstenhater thorstenhater commented Oct 9, 2023

By careful re-analysis of the diffusion solver we found that the coeffiecients of the LHS need an additional factor of the CV Volume.

Scopes be creeping. During the analysis it became apparent that the during coupling to the cable equation proper raises at lot of potential problems.

Conversion of ionic current densities $i_X$ to mass transfer is $\dot c_X = \frac{i_XA}{q F V}$ with volume, area, Faraday's constant, and charge. Now, people are interested in using neutral species $n$, ie $q=0$ for which we also should expect $i_n=0$. Yet we need to special case on this (and assert zero current) to avoid ill-defined division. That's the smaller problem.

The more worrying issue is this:
We construct a coupling term from the cable equation to the ionic diffusion via $\dot c_X = \frac{i_XA}{q F V}$. Yet, there's no backreaction to the cable equation, unless the user explicitly constructs it. How? Well, the way ions should feed back to the voltage is this:
iX = g(U-eX)
this is the conductance model and should be found in an NMODL file and
eX $\sim \ln\frac{X_i}{X_o}$
which is the Nernst equation found, again, in a builtin NMODL file. But, for technical reasons we had to invent a special diffusive concentration Xd which is not identical to Xi. For proper behaviour, we should have used Xd instead of Xi in the Nernst term above. So, a non-standard Nernst module needs to be used.

This brings me to the great change in this context: remove the offending term and carefully document how to retrofit it in NMODL and add the proper Nernst model, too.

fixes #2145
requires #2209

jlubo and others added 30 commits August 24, 2023 15:55
…pointing to the fact that the diffusion across segments of different radius is still erroneous; see arbor-sim#2145)
…pointing to the fact that the diffusion across segments of different radius is still erroneous; see arbor-sim#2145)
…ent_radii' again (for as long as the related fix is not merged)
@thorstenhater
Copy link
Contributor Author

@jlubo I finished testing on our GPU cluster and everything looks fine now. If you could help to repeat this on your cluster and clarify the scaling we can merge this and release 0.11 including the fixes.

@thorstenhater thorstenhater requested a review from jlubo January 16, 2025 08:54
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 this pull request may close these issues.

Flaws in the diffusion process Units and behavior of diffusive particles
2 participants