Becksteinlab / kda

Python package used for the analysis of biochemical kinetic diagrams.
GNU General Public License v3.0
3 stars 1 forks source link

API: Build expressions with `sympy` directly, remove `expressions.py` #14

Open nawtrey opened 4 years ago

nawtrey commented 4 years ago

The idea is to use the new precedent set by kda.calculate_thermo_force() of incorporating the string-to-SymPy conversions into the calculation functions themselves instead of having separate functions for this. The original idea was to give the user the maximum amount of flexibility possible, but now it seems giving access to the string versions of functions doesn't really add any value. It would be simpler from a user's perspective to be given the SymPy functions that can then be substituted/simplified and converted to lambda functions.

nawtrey commented 4 years ago

Another reason to do this is that it is not straightforward for a user to go from a set of string functions to the final SymPy function they are looking for, but it is easy to go from a SymPy function back to a string. Simply cast any SymPy function as a string and you will get back your string function.

nawtrey commented 6 months ago

Perhaps a more ambitious goal would be to handle all the expression creation with sympy (instead of constructing expressions as strings and parsing them with sympy.parser.parse_expr()). This is actually what they recommend in their "best practices". My only concern would be an increase in run time or memory usage, but I think it would be easy to make the change and compare.

nawtrey commented 1 month ago

This is actually a simple conversion in terms of code and only comes at a small cost to performance. There are some key benefits to doing this, some of which are highlighted by the SymPy folks themselves (see here).

I don't think I need to provide much motivation here, but basically we should be building our expressions in terms of sympy.symbols from the very beginning. This is the pythonic way to build algebraic expressions (they discuss some pitfalls of using parse_expr on string functions) and allows us to enforce assumptions about our parameters (i.e. k12, k21 = symbols('k_{12} k_{21}', real=True, positive=True)). There are some smaller benefits, like the ability to retrieve the list of variables from an expression very easily.

Once converted, we can likely remove the entire expressions.py module since it will be completely unnecessary. Even longer term it would be nice to switch everything to use symbolic=True as the default, and remove all numeric calculations from our code. Numeric evaluations can always be performed on KDA expressions once they have been lambified, and that should be the encouraged procedure if numerical outputs are needed.

Unfortunately we will have to wait to implement these changes to make sure master is able to run the kda manuscript code for some time.