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

Timepoint-specific noise parameters not detected #1214

Closed
yannikschaelte opened this issue Aug 20, 2020 · 9 comments
Closed

Timepoint-specific noise parameters not detected #1214

yannikschaelte opened this issue Aug 20, 2020 · 9 comments
Labels
bug PEtab PEtab-import related python

Comments

@yannikschaelte
Copy link
Member

With @lcontento .

In a PEtab test model (attached), we use a noiseFormula sqrt(alpha * noiseParameter1_obs1) with measurement table

observableId    simulationConditionId   time    measurement     noiseParameters
obs1    condition1      1.0     3       1.0
obs1    condition1      2.0     -5      0.25

Thus, we effectively have a time-dependent sigma. Had we used formula sqrt(noiseParameter2_obs1 * noiseParameter1_obs1) with replacement 1.0;alpha and 0.25;alpha in the measurement table, this would have been detected. Like this, however AMICI does not see any problem and simulates the model wrongly. Namely,

from pypesto.petab import PetabImporter
importer = PetabImporter.from_yaml('test.yaml')
objective = importer.create_problem().objective

print(objective(importer.petab_problem.x_nominal_free_scaled))
print(objective([4]))
print(objective([5]))

>>> 204.95158270528947
>>> 204.95158270528947
>>> 204.95158270528947

ignores any parameterization and implicitly uses another noise variable (which?).

Suggestions

  1. Capture such cases and raise a warning (should be possible by evaluating the final noise formula after replacements)
  2. Implement timepoint specific noise parameters :) .
@FFroehlich
Copy link
Member

AMICI itself allows for the specification of timepoint specific sigmas: https://amici.readthedocs.io/en/latest/generated/amici.amici.ExpData.html#amici.amici.ExpData.setObservedDataStdDev

I would assume this function is not used (correctly) in the petab objective?

@yannikschaelte
Copy link
Member Author

AMICI allows for timepoint+parameter dependent sigmas?

Ah, the code was not attached. GitHub should have a warning like mails when "attached" is mentioned. Here it comes: test.zip

@FFroehlich
Copy link
Member

AMICI allows for timepoint+parameter dependent sigmas?

Ah, the code was not attached. GitHub should have a warning like mails when "attached" is mentioned. Here it comes: test.zip

You can specify timepoint dependent sigmas. Additional parameter dependency is not possible.

@yannikschaelte
Copy link
Member Author

Yes, the problem is that in this case the parameter dependency was not recognized by PEtab/AMICI. We have set up a check which raises in case timepoint+parameter dependent sigmas are used, which however did not recognize this case here. So in this case the modeler would assume everything is alright, while actually it is not.

@dweindl dweindl added the PEtab PEtab-import related label Aug 20, 2020
@FFroehlich
Copy link
Member

FFroehlich commented Aug 20, 2020

Makes sense, sorry didn't realise the parameter dependency earlier on. Not straightforward to implement something like that as it probably requires quite a bit of refactoring and a way to specify timepoint-dependencies.

One (inefficient) approach would be to seperate this into two different simulation conditions where noiseParameter2_obs1 is encoded as experimental condition. Alternatively one could use heaviside functions to encode temporal dependency, this is likely going to have scaling issues aswell.

Probably worthwhile adding this to the petab testsuite as I would expect that other tools also have issues with dealing such petab problems.

@yannikschaelte
Copy link
Member Author

yannikschaelte commented Aug 20, 2020

Agreed. I added an issue to the petab test suite for this.

Separating this into different simulation conditions would be not feasible I think, because the real problem has 100 time point units or so, each with their own special parameter. (But there we are currently still investigating alternatives.)

@FFroehlich
Copy link
Member

Thought about this again. I think the best implementation would be to allow specification of sigma derivatives in ExpData. Probably after #931 is adressed 😞 .

@dweindl dweindl removed the new Newly created label Oct 18, 2020
@FFroehlich
Copy link
Member

@yannikschaelte , this should now correctly be detected in the latest AMICI release, right?

@yannikschaelte
Copy link
Member Author

@FFroehlich yes, it is detected by ValueError: Timepoint-specific parameter overrides currently unsupported. now!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug PEtab PEtab-import related python
Projects
None yet
Development

No branches or pull requests

3 participants