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

Draft: Update pituitary gland ODE with better simulator, ODE class and dataframe #94

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

Conversation

WolfByttner
Copy link
Contributor

This is an update to get better ODE parameters, add an ODE class (based on its behaviour) and integrate it more closely with Traja. The new setup generates dataframes to integrate with other Traja functions and work with the resampler, analysis functions.

@codecov
Copy link

codecov bot commented Aug 8, 2021

Codecov Report

Merging #94 (42b948a) into master (f347c8c) will increase coverage by 0.46%.
The diff coverage is 84.61%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #94      +/-   ##
==========================================
+ Coverage   76.28%   76.75%   +0.46%     
==========================================
  Files          26       26              
  Lines        2703     2869     +166     
==========================================
+ Hits         2062     2202     +140     
- Misses        641      667      +26     
Flag Coverage Δ
unittests 76.75% <84.61%> (+0.46%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
traja/dataset/pituitary_gland.py 87.43% <84.61%> (-12.57%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f347c8c...42b948a. Read the comment docs.


@jit
def s_inf(c, k_s):
return c ** 2 / (c ** 2 + k_s ** 2)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please document method


@jit
def x_inf(V, V_x, s_x):
return 1 / (1 + exp((V_x - V) / s_x))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please document method

@Saran-nns Saran-nns self-requested a review November 22, 2021 07:42
@Saran-nns
Copy link
Member

Saran-nns commented Nov 22, 2021

Hi Johan, the pytest fails on the windows due to the length of the function names, i assume. Months ago @JustinShenk requested me to reduce the function names corresponding to ODE problems/models assuming the problem arises due to pep function/attribute naming style. However, we do not know the exact reason for the failure.

I also wish to have these ODE models as a general-purpose package at Traja. At the moment this PR seems to have examples of ODE models exclusively for forecasting the parameters of the pituitary gland. Is there any way we could extract these methods and develop them as a general-purpose package for ODE problems?

Thanks in advance ;)

@@ -3,7 +3,8 @@
import pytest

from traja.dataset import dataset
from traja.dataset.pituitary_gland import create_latin_hypercube_sampled_pituitary_df
from traja.dataset.pituitary_gland import create_latin_hypercube_sampled_pituitary_df, \
pituitary_ori_ode_parameters_Isk_Ibk_Ikir_Icat_Ia_Inav, generate_pituitary_dataset
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @WolfByttner Please rename these methods pituitary_ori_ode_parameters_Isk_Ibk_Ikir_Icat_Ia_Inav. For example something like get_params(*list(pnames),**kwargs) would be ideal. More than that travis CI stuck at those methods for some weird reasons.

@@ -653,3 +654,8 @@ def test_pituitary_gland_latin_hypercube_generator_gives_correct_number_of_sampl
_, num_samples_out = create_latin_hypercube_sampled_pituitary_df(samples=num_samples)

assert num_samples == num_samples_out, "Hypercube sampler returned the wrong number of samples!"


def test_pituitary_gland_random_sampler_generates_valid_dataframes():
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this methods could be simply df_random_sampler

df = compute_pituitary_gland_df_from_parameters(downsample_rate,
gcal, gsk, gk, gbk, gl, kc,
sample_id)
df = compute_pituitary_gland_df_fletcher_from_parameters(downsample_rate,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this method can be compute_fletcher

@Saran-nns
Copy link
Member

@WolfByttner I suggest this PR for traja-research since it is more or less application-specific at the moment. @JustinShenk what do you think?

@JustinShenk
Copy link
Collaborator

JustinShenk commented Dec 2, 2021 via email

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.

None yet

3 participants