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

Use cpdef instead of a wrapper "py_<function>" function where appropriate #45

Open
sclamons opened this issue Jun 8, 2020 · 1 comment
Labels
documentation Software and example documentation enhancement New feature or request

Comments

@sclamons
Copy link
Collaborator

sclamons commented Jun 8, 2020

Bioscrape uses a lot of cdef'ed functions with a wrapper for python, e.g. the cdef function "simulate_model" (which cannot be called from python) wrapped with a python function "py_simulate_model". This is a little clunky, and mysterious to users who aren't familiar with Cython.

Cython has a built-in keyword "cpdef" that makes a function both typed and python-callable. Calling a cpdef function is slower than calling a cdef function (see https://notes-on-cython.readthedocs.io/en/latest/fibo_speed.html), but that only affects the time required to call the function, not to run its contents (see https://stackoverflow.com/questions/49172528/should-i-define-my-cython-function-using-def-cdef-or-cpdef-for-optimal-perform). For functions that only get called a small number of times, like simulate_model, we should use cpdef instead of cdef so we don't have the weird "py_" handle gunking up our function names.

This should be a quick fix, but someone will need to think about which functions need "py_" wrappers (for speed) and which can be cpdef-ed safely (for comprehensibility and convenience).

@sclamons sclamons added enhancement New feature or request documentation Software and example documentation labels Jun 8, 2020
@WilliamIX
Copy link
Collaborator

The simulate functions are actually called fairly frequently, especially with interacting lineages. In addition, simulate_single_cell is written in such a way as to reuse memory buffers, which have to be initializes separately. This is very important for performance.

I also think that having py_simulate functions which are not connected to classes is helpful so we can use Bioscrape for scripting without instantiating the classes.

I understand your confusion though. One possible thought is to move all the py_simulate functions into a separate file called wrappers. Would this help make things clearer?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Software and example documentation enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants