ComputationalPhysiology / gotran

Library for declaring and translating ODEs
http://computationalphysiology.github.io/gotran/
GNU Lesser General Public License v3.0
2 stars 1 forks source link

Rush-Larsen schemes: Remove unneccessary division by zero checks #7

Closed KGHustad closed 2 years ago

KGHustad commented 3 years ago

Linearised expressions for typical gate variables take the form 1/a, which cannot be zero, so we can remove the runtime check when dividing by the linearised expression in the generated code.

It was somewhat tricky to make sympy perform this check without any substantial increase in the execution time, as sympy supports a number of expensive checks for the numerator being non-zero. It's better to make the check inexpensive, and then we can fall back to generating the extra runtime check, even if it may be possible to prove that division by zero cannot possibly occur.

The final implementation does not seem to increase the execution time of gotran, and in addition to making the generated code simpler, it should lead to slightly faster execution time in the generated code, .

KGHustad commented 3 years ago

As you mention, it may be a good idea to refactor some of the code.

At the moment we have three Rush-Larsen-based methods:

I think all three could be based on the same class. The difference lies in which state variables are using FE or GRL1. In the case of classical Rush-Larsen, we need to analyse the state expressions to determine which scheme to use.

finsberg commented 2 years ago

@KGHustad What is the status on this one, can we merge it into master?

KGHustad commented 2 years ago

I wanted to refactor the RL classes, but I didn't have time, so I think we can merge this PR and rather open a new one if we decide to refactor.