Feature validation profiles closes #94 #658

jvanhoefer · 2021-05-07T12:28:41Z

Provides basic functionality to compute validation significances, as in Kreutz et al. 2012.

Basically one has to provide a pypesto.Problem encoding the fitting of the full data set as well as pypesto.results for the training and the full data set. The functionality then computes the significance of the validation data set.

For a whole "validation profile" (i.e. a profile of a new data point in the "data space"...) one would need to specify the concept of "data" in more detail. Quite possible to also do this via one more layer around the functionality provided in this PR (i.e. write a loop, that gradually in-/decreases the new measurement value and computes the validation_profile_significance...).

codecov-commenter · 2021-05-07T12:37:19Z

Codecov Report

Merging #658 (a3560b5) into develop (69e834d) will decrease coverage by 0.10%.
The diff coverage is 71.05%.

❗ Current head a3560b5 differs from pull request most recent head bca4092. Consider uploading reports for the commit bca4092 to get more accurate results

@@             Coverage Diff             @@
##           develop     #658      +/-   ##
===========================================
- Coverage    89.72%   89.61%   -0.11%     
===========================================
  Files           94       95       +1     
  Lines         6083     6117      +34     
===========================================
+ Hits          5458     5482      +24     
- Misses         625      635      +10

Impacted Files	Coverage Δ
pypesto/problem.py	`91.57% <44.44%> (-0.34%)`	⬇️
pypesto/profile/validation_intervals.py	`78.57% <78.57%> (ø)`
pypesto/profile/__init__.py	`100.00% <100.00%> (ø)`
pypesto/engine/multi_process.py	`92.59% <0.00%> (-7.41%)`	⬇️
pypesto/objective/amici_util.py	`84.21% <0.00%> (-0.88%)`	⬇️

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 69e834d...bca4092. Read the comment docs.

pypesto/profile/validation_intervals.py

yannikschaelte · 2021-05-07T12:40:50Z

pypesto/profile/validation_intervals.py

+        lsq_objective: float = False
+) -> float:
+    """
+    Computes the significance of an validation experiment as described in


Suggested change

Computes the significance of an validation experiment as described in

Computes the significance of a validation experiment as described in

yannikschaelte · 2021-05-07T12:41:30Z

pypesto/profile/validation_intervals.py

+) -> float:
+    """
+    Computes the significance of an validation experiment as described in
+    Kreutz et al. BMC Systems Biology 2012.


an actual reference would be good to make it easier to find in the future. See https://github.com/ICB-DCM/pyABC/blob/main/pyabc/inference/smc.py#L61 (and maybe use an actual url 🙈 )

pypesto/profile/validation_intervals.py

yannikschaelte · 2021-05-07T12:43:28Z

pypesto/profile/validation_intervals.py

+    ----------
+    problem_full_data:
+        pypesto.problem, such that the objective is the
+        negative-log-likelihood of the training and validation data set.


what would be if there is a Bayesian prior defined?

I'm wondering whether it's better to define the "training problem" and the "full problem" or rather the "training problem" and the "validation problem". While I see that it's handy just to call an objective function of the full problem, it seems 1. hard to "subtract" the training problem from the full problem, if at some point it will be necessary to use the validation problem itself... moreover, it might 2. be less intuitive for the user to provide a "training" and a "full" problem rather than a "training" and a "validation" problem...
2nd is just a guess, the actual argument would be 1. What do you think about this?

yannikschaelte · 2021-05-07T12:52:42Z

pypesto/profile/validation_intervals.py

+                result_training_data.optimize_result.get_for_key('x')[0]))
+
+    if nllh_new > nllh_old:
+        raise RuntimeError("Fitting of the full data set provided a worse fit "


I would raise errors only in the case of programmatic errors, e.g. wrong inputs, unexpected values, ... which should not be possible with correct usage of the tool. as this is "only" a bad situation having nothing to do with the code, but the application, I would logger.warning(...).

Just to correctly understand this: Are you raising an error if the optimized nllh value is worse than the value for the vector, which was used to initialize the optimization? Can this happen at all? 🤔

yannikschaelte · 2021-05-07T12:54:15Z

pypesto/profile/validation_intervals.py

+                           "result_full_data or running the fitting from the "
+                           "best parameters found from the training data.")
+
+    if lsq_objective:


comment on what happens here, i.e. sf = survival function, and how that should be interpreted.

yannikschaelte · 2021-05-07T12:56:12Z

test/profile/test_validation_intervals.py

+        cls.result_all_data = optimize.minimize(cls.problem_all_data,
+                                                n_starts=5)
+
+    def test_validation_intervals(self):


yannikschaelte · 2021-05-07T12:57:39Z

pypesto/profile/validation_intervals.py

+    """
+    Computes the significance of an validation experiment as described in
+    Kreutz et al. BMC Systems Biology 2012.
+


I would appreciate a longer / more informative docstring here what "significance of a validation experiment" is, as e.g. I have no idea and would need to look up the publication to see whether this function is what I'm looking for ...

yannikschaelte · 2021-05-07T12:58:43Z

test/profile/test_validation_intervals.py

+
+        # fit with refitting inside function
+        profile.validation_profile_significance(self.problem_all_data,
+                                                self.result_training_data)


is it somehow possible to test whether the output makes sense? i.e. the returned values are as expected when fitting on full data from the same model, vs they give point to problems when that's not the case?

pypesto/profile/validation_intervals.py

dweindl · 2021-05-10T09:00:06Z

test/profile/test_validation_intervals.py

+"""
+
+import numpy as np
+import unittest


Would avoid mixing unittestand pytest to keep things simpler, but not sure what the pypesto policy is there.

Haha, my policy would always be to use pytest instead of the class-syntax, which I find hard to read. Would be possible here for sure, but all would be fine for me.

dweindl · 2021-05-10T09:03:28Z

test/profile/test_validation_intervals.py

+        return (x[0]-d)**2
+
+    def grad(x):
+        return 2 * (x-d)


x vs x[0]

assert x.size == 1?

hm... agreeing: either ensure using np.ndarray, then this thing goes through also for vectors, or enforce x to be a number... Implicitly, this code assumes x to be an iterable object, while it strictly assumes (implicitly) that d is a number... I find that weird.

paulstapor

I just added comments where there weren't any so far, and @yannikschaelte made this job easy... :D Overall, I think that's fine, I'm just wondering because this is somewhat orthogonal to the prediction functionality. This is okay per se, I'm simply wondering how to best combine things if one wants to actually compute prediction profiles from a pyPESTO prediction...

paulstapor · 2021-05-11T17:41:37Z

pypesto/profile/validation_intervals.py

+    ----------
+    problem_full_data:
+        pypesto.problem, such that the objective is the
+        negative-log-likelihood of the training and validation data set.


I'm wondering whether it's better to define the "training problem" and the "full problem" or rather the "training problem" and the "validation problem". While I see that it's handy just to call an objective function of the full problem, it seems 1. hard to "subtract" the training problem from the full problem, if at some point it will be necessary to use the validation problem itself... moreover, it might 2. be less intuitive for the user to provide a "training" and a "full" problem rather than a "training" and a "validation" problem...
2nd is just a guess, the actual argument would be 1. What do you think about this?

paulstapor · 2021-05-11T17:51:54Z

test/profile/test_validation_intervals.py

+        return (x[0]-d)**2
+
+    def grad(x):
+        return 2 * (x-d)


hm... agreeing: either ensure using np.ndarray, then this thing goes through also for vectors, or enforce x to be a number... Implicitly, this code assumes x to be an iterable object, while it strictly assumes (implicitly) that d is a number... I find that weird.

paulstapor · 2021-05-11T17:55:13Z

pypesto/profile/validation_intervals.py

+                result_training_data.optimize_result.get_for_key('x')[0]))
+
+    if nllh_new > nllh_old:
+        raise RuntimeError("Fitting of the full data set provided a worse fit "


Just to correctly understand this: Are you raising an error if the optimized nllh value is worse than the value for the vector, which was used to initialize the optimization? Can this happen at all? 🤔

paulstapor · 2021-05-11T17:57:27Z

pypesto/profile/validation_intervals.py

+        problem = Problem(
+            objective=problem_full_data.objective,
+            lb=problem_full_data.lb,
+            ub=problem_full_data.ub,
+            dim_full=problem_full_data.dim_full,
+            x_fixed_indices=problem_full_data.x_fixed_indices,
+            x_fixed_vals=problem_full_data.x_fixed_vals,
+            x_guesses=x_0,
+            startpoint_method=problem_full_data.startpoint_method,
+            x_names=problem_full_data.x_names,
+            x_scales=problem_full_data.x_scales,
+            x_priors_defs=problem_full_data.x_priors,
+            lb_init=problem_full_data.lb_init,
+            ub_init=problem_full_data.ub_init)


Wouldn't another possible solution just be to yet use deepcopy and then overwrite the x_guesses data member with x0? To me, this feels much more handy (as just as stable as what Yannik suggested) than adding a lot of interface to the problem class...

yannikschaelte

looks good to me

…DCM/pyPESTO into feature_validation_profiles

jvanhoefer added 2 commits May 5, 2021 19:19

initial commit Validation Intervals

6609930

add tests for validation intervals

d09756d

jvanhoefer requested review from paulstapor, dweindl and yannikschaelte May 7, 2021 12:28

yannikschaelte requested changes May 7, 2021

View reviewed changes

yannikschaelte assigned jvanhoefer May 7, 2021

dweindl reviewed May 10, 2021

View reviewed changes

paulstapor approved these changes May 11, 2021

View reviewed changes

jvanhoefer added 2 commits May 12, 2021 16:35

fix comments, extend docu

bee32ca

fix factor 2

f9866f7

yannikschaelte approved these changes May 12, 2021

View reviewed changes

yannikschaelte and others added 5 commits May 12, 2021 17:12

Merge branch 'develop' into feature_validation_profiles

9533c7b

Merge branch 'develop' into feature_validation_profiles

a3560b5

fix link in doc string...

eb39c4d

Merge branch 'feature_validation_profiles' of https://github.com/ICB-…

bca4092

…DCM/pyPESTO into feature_validation_profiles

fix link in doc string...

6518b94

jvanhoefer merged commit 6acc530 into develop May 12, 2021

jvanhoefer deleted the feature_validation_profiles branch May 12, 2021 17:35

yannikschaelte mentioned this pull request May 17, 2021

Release 0.2.6 #655

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature validation profiles closes #94 #658

Feature validation profiles closes #94 #658

jvanhoefer commented May 7, 2021

codecov-commenter commented May 7, 2021 •

edited

Loading

yannikschaelte May 7, 2021

yannikschaelte May 7, 2021

yannikschaelte May 7, 2021

paulstapor May 11, 2021

yannikschaelte May 7, 2021

paulstapor May 11, 2021

yannikschaelte May 7, 2021

yannikschaelte May 7, 2021

yannikschaelte May 7, 2021

yannikschaelte May 7, 2021

dweindl May 10, 2021

yannikschaelte May 10, 2021

dweindl May 10, 2021

paulstapor May 11, 2021

paulstapor left a comment

paulstapor May 11, 2021

paulstapor May 11, 2021

paulstapor May 11, 2021

paulstapor May 11, 2021

yannikschaelte left a comment

	Computes the significance of an validation experiment as described in
	Computes the significance of a validation experiment as described in

Feature validation profiles closes #94 #658

Feature validation profiles closes #94 #658

Conversation

jvanhoefer commented May 7, 2021

codecov-commenter commented May 7, 2021 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paulstapor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yannikschaelte left a comment

Choose a reason for hiding this comment

codecov-commenter commented May 7, 2021 •

edited

Loading