Skip to content

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 ;)

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.

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.

6 participants