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

Problems with accessing dv/dS (Jacobian) #910

Closed
matthiaskoenig opened this issue Nov 8, 2021 · 23 comments
Closed

Problems with accessing dv/dS (Jacobian) #910

matthiaskoenig opened this issue Nov 8, 2021 · 23 comments

Comments

@matthiaskoenig
Copy link
Collaborator

matthiaskoenig commented Nov 8, 2021

Hi all,
I want to access the changes in reaction rates depending on species.
As I remember this is the information available via the Jacobian, i.e. dv_i/dS_k is the J_ik.
image

So I tried to access the reduced Jabobian, but get a matrix which has species ids for rows and columns (and not rate ids for rows and species ids for columns)?

              apap_ext,       apap,        napqi,       CYP2E1
apap_ext [[ -0.0284312,  0.0927807,            0,            0],
apap      [  0.0284312,  -0.099167,            0, -3.40902e-05],
napqi     [          0, 0.00638628, -0.000799083,  3.40902e-05],
CYP2E1    [          0,          0,            0,            0]]

Is this correct? I would expect reaction ids in the rows here? What do the entries mean in this square matrix? And how would I access the changes of reaction rates with species?

When trying to access the full Jacobian, I get

python: /__w/1/s/source/rrRoadRunner.cpp:3765: double rr::RoadRunner::getUnscaledSpeciesElasticity(int, int): Assertion `std::abs(originalConc - tmp) < 1e-13' failed.

Process finished with exit code 134 (interrupted by signal 6: SIGABRT)

so that I cannot access the values.

Code and model attached:
apap_core.zip

import roadrunner

from pkdb_models.models.acetaminophen_pm import MODEL_BASE_PATH
from pkdb_models.models.acetaminophen_pm.models import apap_liver_core

# -------------------------------------------------------------------------------------
# load model
# -------------------------------------------------------------------------------------
# model_path = MODEL_BASE_PATH / f"{apap_liver_core.mid}_{apap_liver_core.version}.xml"
# change this to the following
model_path = "apap_liver_core_9.xml"
print(model_path)

# roadrunner instance for local point
r = roadrunner.RoadRunner(str(model_path))

# -------------------------------------------------------------------------------------
# selections
# -------------------------------------------------------------------------------------
# in a first step it must be defined what should be simulated in the model.
# The following sets this selection
r.timeCourseSelections = [
    "time",
    # local concentrations
    "[apap_ext]",
    "[apap]",
    "[napqi]",

    # sinks/sources
    "d_apap_ext__dt",
    # tangents
    "d_vapap_ext__d_apap_ext",
    
    # other readouts
    "necrosis",
    "CYP2E1",

    # rate of change
    "apap_ext'",  # [mmole/min]
    "apap'",  # [mmole/min]
    "napqi'",  # [mmole/min]

]

# -------------------------------------------------------------------------------------
# set integrator settings
# -------------------------------------------------------------------------------------
# Some integrator settings are required to handle the very small volumes.

# set tolerances for very small FEM volumes
integrator: roadrunner.Integrator = r.integrator
integrator.setValue("absolute_tolerance", 1e-14)
integrator.setValue("relative_tolerance", 1e-14)

# -------------------------------------------------------------------------------------
# set values
# -------------------------------------------------------------------------------------
# volumes based on FEM point
# Vext is fluid phase of the point, Vcell is the fat + cell phase
# e.g. if you have
vol_point = 0.1  # [liter]
f_fluid = 0.2  # [dimensionless]
f_fat = 0.3  # [dimensionless]
f_tissue = 1 - f_fluid - f_fat

# setting volumes
r.setValue('Vext', vol_point * f_fluid)  # [liter]
r.setValue('Vli', vol_point * (f_fat + f_tissue))  # [liter]

# protein amount based on position
# (varied with position of grid point between 0.0 periportal and 1.0 pericentral)
r.setValue('CYP2E1', 1.0)  # [dimensionless] pericentral point

# concentrations of apap in fluid phase (local concentration of you fluid phase)
r.setValue('[apap_ext]', 1.0)  # [mM] = [mmole/liter]

# -------------------------------------------------------------------------------------
# simulate timestep
# -------------------------------------------------------------------------------------
time = 0.0  # [min]
delta_time = 0.1  # [min]
# simulate a step
s = r.simulate(start=time, end=time+delta_time, steps=1)
print(s)

# -------------------------------------------------------------------------------------
# access results
# -------------------------------------------------------------------------------------
# source and sink terms
print("d_apap_ext__dt", s["d_apap_ext__dt"], "[mmole/min]")

# tangents dv/dc = [mmole/min]/[mmole/liter]
print("d_vapap_ext__d_apap_ext", s["d_vapap_ext__d_apap_ext"], "[l/mmole]")


# necrosis
print("necrosis", s["necrosis"], "[dimensionless]")
# internal concentrations
print("[napqi]", s["[napqi]"], "[mM]")
print("[apap]", s["[apap]"], "[mM]")

Jred = r.getReducedJacobian()
print(Jred)

J = r.getFullJacobian()
print(J)
@hsauro
Copy link

hsauro commented Nov 8, 2021 via email

@matthiaskoenig
Copy link
Collaborator Author

@hsauro Thanks. The r.getuEE ("v", "s") syntax worked for me, i.e., using r.getuEE("APAPIM", "apap_ext") for the model. But this requires calling this after every time step.
I could not get it working via the selections, i.e. trying to add 'uEE("APAPIM", "apap_ext")' or 'uEE(APAPIM, apap_ext)' to r.timeCourseSelections so that I have the unscaled elasticity timecourse in my simulation results. Both result in python in a

  File "/home/mkoenig/envs/pkdb_models/lib/python3.8/site-packages/roadrunner/roadrunner.py", line 3132, in _setSelections
    return _roadrunner.RoadRunner__setSelections(self, selections)
RuntimeError: invalid selection std::string uEE("APAPIM", "apap_ext")
...
  File "/home/mkoenig/envs/pkdb_models/lib/python3.8/site-packages/roadrunner/roadrunner.py", line 3132, in _setSelections
    return _roadrunner.RoadRunner__setSelections(self, selections)
RuntimeError: invalid selection std::string uEE(APAPIM, apap_ext)

This should be working from the description in
http://sys-bio.github.io/roadrunner/docs-build/selecting_values.html
It would be great if the EE, uEE, CC and uCC could be added to the selection.

The model was attached in the original comment.

@hsauro
Copy link

hsauro commented Nov 8, 2021 via email

@luciansmith
Copy link

I did not reproduce your errors with your models on my machine. My results were:

apap_liver_core_9.xml
    time, [apap_ext],   [apap],     [napqi], d_apap_ext__dt, d_vapap_ext__d_apap_ext, necrosis, CYP2E1,  apap_ext',     apap',      napqi'
 [[    0,          1,        0,           0,     -0.0533333,               0.0533333,        0,      1, -0.0533333, 0.0533333,           0],
  [  0.1,   0.760385, 0.059617, 0.000286703,     -0.0424707,                0.060606,        0,      1, -0.0424707, 0.0420446, 0.000425898]]

d_apap_ext__dt [-0.05333333 -0.04247071] [mmole/min]
d_vapap_ext__d_apap_ext [0.05333333 0.06060598] [l/mmole]
necrosis [0. 0.] [dimensionless]
[napqi] [0.        0.0002867] [mM]
[apap] [0.         0.05961704] [mM]
              apap_ext,       apap,        napqi,       CYP2E1
apap_ext [[ -0.0284312,  0.0927807,            0,            0],
apap      [  0.0284312,  -0.099167,            0, -3.40902e-05],
napqi     [          0, 0.00638628, -0.000799083,  3.40902e-05],
CYP2E1    [          0,          0,            0,            0]]

               apap_ext,      apap,      napqi,      CYP2E1
apap_ext [[ -0.00477214, 0.0499607,          0,           0],
apap      [  0.00477214,   -0.3186,          0, -0.00414565],
napqi     [           0,   0.26864, -0.0146826,  0.00414565],
CYP2E1    [           0,         0,          0,           0]]

I also discovered that to get the unscaled elasticity coefficient, the syntax is:

'uec(APAPIM, apap_ext)'

However, if I add this to your time course selections list in the program you provided, it fails, giving the following error:

"The second argument to a metabolic control coefficient selection, apap_ext, must be either a global parameter, boundary species or conserved sum"

Given that 'getuEE()' works with those arguments, this seems like a bug in roadrunner to me, perhaps having to do with it checking whether it's a metabolic control coefficient instead of an unscaled elasticity coefficient? At any rate, I'll look into it.

@hsauro
Copy link

hsauro commented Nov 9, 2021 via email

@matthiaskoenig
Copy link
Collaborator Author

@luciansmith Thanks for looking into this.
I also think this is just a bug/issue with the selections.

Did you change the volumes as in the scripts before calculating the Jacobian? If I just run the model with the default volumes I do not get the errors in the Jacobian.

In addition I set stricter tolerances for smaller volumes (absolute_tolerance and relative_tolerance). These tolerances should also be used in the Jacobian cutoffs. I.e. if I have

integrator.setValue("absolute_tolerance", 1e-14)
integrator.setValue("relative_tolerance", 1e-14)

It is strange to get an error
Assertion std::abs(originalConc - tmp) < 1e-13'This looks like a hard-coded cutoff which should in reality be based onabsolute_toleranceand probably alsorelative_tolerance`.

@luciansmith
Copy link

So! A few things:

  • I was unable to reproduce your original problem, but I don't understand how you could possibly get your original error message anyway: the error message is in an 'assert' statement that only executes in debug mode. Did you manage to get a version of roadrunner compiled in debug mode to run in Python?
  • However, I did change the assert in question to be relative rather than absolute. Hopefully that'll help whatever problem you were hitting either way. (The asserts were just sanity checks, and did not involve actual calculation of the Jacobian.)
  • I did find other bugs in the process, which are now fixed. The upshot is that in the fixed version of the code, you should be able to use 'uec(APAPIM, apap_ext)' to get the desired elasticity as part of the normal simulation run.

The fixes are incorporated into #911

Here's the updated python script I used, and the results I get from it:

import roadrunner

# -------------------------------------------------------------------------------------
# load model
# -------------------------------------------------------------------------------------
# model_path = MODEL_BASE_PATH / f"{apap_liver_core.mid}_{apap_liver_core.version}.xml"
# change this to the following
model_path = "apap_liver_core_9.xml"
print(model_path)

# roadrunner instance for local point
r = roadrunner.RoadRunner(str(model_path))

# -------------------------------------------------------------------------------------
# selections
# -------------------------------------------------------------------------------------
# in a first step it must be defined what should be simulated in the model.
# The following sets this selection
r.timeCourseSelections = [
    "time",
    # local concentrations
    "[apap_ext]",
    "[apap]",
    "[napqi]",

    # sinks/sources
    "d_apap_ext__dt",
    # tangents
    "d_vapap_ext__d_apap_ext",
    
    # other readouts
    "necrosis",
    "CYP2E1",

    # rate of change
    "apap_ext'",  # [mmole/min]
    "apap'",  # [mmole/min]
    "napqi'",  # [mmole/min]
    
    #uec:
    'uec(APAPIM, apap_ext)'

]

# -------------------------------------------------------------------------------------
# set integrator settings
# -------------------------------------------------------------------------------------
# Some integrator settings are required to handle the very small volumes.

# set tolerances for very small FEM volumes
integrator: roadrunner.Integrator = r.integrator
integrator.setValue("absolute_tolerance", 1e-14)
integrator.setValue("relative_tolerance", 1e-14)

# -------------------------------------------------------------------------------------
# set values
# -------------------------------------------------------------------------------------
# volumes based on FEM point
# Vext is fluid phase of the point, Vcell is the fat + cell phase
# e.g. if you have
vol_point = 0.1  # [liter]
f_fluid = 0.2  # [dimensionless]
f_fat = 0.3  # [dimensionless]
f_tissue = 1 - f_fluid - f_fat

# setting volumes
r.setValue('Vext', vol_point * f_fluid)  # [liter]
r.setValue('Vli', vol_point * (f_fat + f_tissue))  # [liter]

# protein amount based on position
# (varied with position of grid point between 0.0 periportal and 1.0 pericentral)
r.setValue('CYP2E1', 1.0)  # [dimensionless] pericentral point

# concentrations of apap in fluid phase (local concentration of you fluid phase)
r.setValue('[apap_ext]', 1.0)  # [mM] = [mmole/liter]

# -------------------------------------------------------------------------------------
# simulate timestep
# -------------------------------------------------------------------------------------
time = 0.0  # [min]
delta_time = 0.1  # [min]
# simulate a step
s = r.simulate(start=time, end=time+delta_time, steps=1)
print(s)

# -------------------------------------------------------------------------------------
# access results
# -------------------------------------------------------------------------------------
# source and sink terms
print("d_apap_ext__dt", s["d_apap_ext__dt"], "[mmole/min]")

# tangents dv/dc = [mmole/min]/[mmole/liter]
print("d_vapap_ext__d_apap_ext", s["d_vapap_ext__d_apap_ext"], "[l/mmole]")


# necrosis
print("necrosis", s["necrosis"], "[dimensionless]")
# internal concentrations
print("[napqi]", s["[napqi]"], "[mM]")
print("[apap]", s["[apap]"], "[mM]")

Jred = r.getReducedJacobian()
print(Jred)

J = r.getFullJacobian()
print(J)

print("unscaled elasticity of APAPIM with respect to apap_ext:", r['uec(APAPIM, apap_ext)'])

And the results:

apap_liver_core_9.xml
    time, [apap_ext],   [apap],     [napqi], d_apap_ext__dt, d_vapap_ext__d_apap_ext, necrosis, CYP2E1,  apap_ext',     apap',      napqi', uec(APAPIM, apap_ext)
 [[    0,          1,        0,           0,     -0.0533333,               0.0533333,        0,      1, -0.0533333, 0.0533333,           0,             0.0177777],
  [  0.1,   0.760385, 0.059617, 0.000286703,     -0.0424707,                0.060606,        0,      1, -0.0424707, 0.0420446, 0.000425898,             0.0284312]]

d_apap_ext__dt [-0.05333333 -0.04247071] [mmole/min]
d_vapap_ext__d_apap_ext [0.05333333 0.06060598] [l/mmole]
necrosis [0. 0.] [dimensionless]
[napqi] [0.        0.0002867] [mM]
[apap] [0.         0.05961704] [mM]
              apap_ext,       apap,        napqi,       CYP2E1
apap_ext [[ -0.0284312,  0.0927807,            0,            0],
apap      [  0.0284312,  -0.099167,            0, -3.40902e-05],
napqi     [          0, 0.00638628, -0.000799083,  3.40902e-05],
CYP2E1    [          0,          0,            0,            0]]

               apap_ext,      apap,      napqi,      CYP2E1
apap_ext [[ -0.00477214, 0.0499607,          0,           0],
apap      [  0.00477214,   -0.3186,          0, -0.00414565],
napqi     [           0,   0.26864, -0.0146826,  0.00414565],
CYP2E1    [           0,         0,          0,           0]]

unscaled elasticity of APAPIM with respect to apap_ext: 0.028431165361284736

@matthiaskoenig
Copy link
Collaborator Author

@luciansmith
thanks for the fix. Just testing this with the azure artifacts and closing this if everything works.

I was unable to reproduce your original problem, but I don't understand how you could possibly get your original error message anyway: the error message is in an 'assert' statement that only executes in debug mode. Did you manage to get a version of roadrunner compiled in debug mode to run in Python?

I work with the libroadrunner and libroadrunner-experimental packages or download arzure artifacts for the develop branch (roadrunner-ManyLinux2014-py38 or py39). I never build from scratch. So the only way the debug mode is selected is if it is selected in your pip releases for linux (or the azure pipeline).
See here:
https://dev.azure.com/TheRoadrunnerProject/roadrunner/_build/results?buildId=1102&view=artifacts&pathAsName=false&type=publishedArtifacts

Why are the reduced Jacobian and Jacobian different?
I thought the reduced Jacobian is just removing the conservation rules. In this model nothing is removed so the Jacobian and Reduced Jacobian should be numerically identical. But:

              apap_ext,       apap,        napqi,       CYP2E1
apap_ext [[ -0.0284312,  0.0927807,            0,            0],
apap      [  0.0284312,  -0.099167,            0, -3.40902e-05],
napqi     [          0, 0.00638628, -0.000799083,  3.40902e-05],
CYP2E1    [          0,          0,            0,            0]]

               apap_ext,      apap,      napqi,      CYP2E1
apap_ext [[ -0.00477214, 0.0499607,          0,           0],
apap      [  0.00477214,   -0.3186,          0, -0.00414565],
napqi     [           0,   0.26864, -0.0146826,  0.00414565],
CYP2E1    [           0,         0,          0,           0]]

Or is the reduced Jacobian something different?

@hsauro
Copy link

hsauro commented Nov 9, 2021 via email

@hsauro
Copy link

hsauro commented Nov 9, 2021 via email

@matthiaskoenig
Copy link
Collaborator Author

@hsauro
Yes. Exactly same state. Basically two calls one after the other without any simulations in between.

@luciansmith
I just tried this with the azure artifacts and now get the following SIGABRT when trying J = r.getFullJacobian()

python: /__w/1/s/source/rrRoadRunner.cpp:3765: double rr::RoadRunner::getUnscaledSpeciesElasticity(int, int): Assertion `originalConc==tmp || (tmp != 0 && std::abs(originalConc/tmp - 1) < 1e-6)' failed.

So it seems the Azure wheels are build with debug mode on. This should probably be changed.

@luciansmith
Copy link

Aha! OK, looks like our azure build didn't set the build type to 'release' for the manylinux builds. Thank you!

However, now I'm wondering if this was fortuitous, because it indicates that the sanity checks have actually failed. I'll investigate further. @hsauro , if I need help somewhere I'll ping you and set up a zoom call. Thanks!

@luciansmith
Copy link

Also, @hsauro : as you check things, try changing some of the values with 'setValue' before calculating the Jacobians. My guess is that one of our routines doesn't take that into account, somehow.

@luciansmith
Copy link

Oh, and @matthiaskoenig : are you OK with your model going into our code as a test? I guess it's already available publicly here on this issue, but that's a bit more obscure than source code.

@hsauro
Copy link

hsauro commented Nov 9, 2021 via email

@hsauro
Copy link

hsauro commented Nov 9, 2021 via email

@hsauro
Copy link

hsauro commented Nov 9, 2021 via email

@matthiaskoenig
Copy link
Collaborator Author

@luciansmith Please use the model as test model.

@hsauro
Copy link

hsauro commented Nov 9, 2021 via email

@hsauro
Copy link

hsauro commented Nov 10, 2021 via email

luciansmith added a commit that referenced this issue Nov 10, 2021
@luciansmith
Copy link

OK! This is now fixed with #912

The problem was that the compartment volumes weren't being synced with the species values when trying to deal with conserved moieties. If you didn't have to worry about that (i.e. the reduced form) there was no error. But now, both should match!

Wheels available at https://dev.azure.com/TheRoadrunnerProject/roadrunner/_build/results?buildId=1103&view=artifacts&pathAsName=false&type=publishedArtifacts

As a bonus, the manylinux build is now compiled in release mode, which should make it faster (though I don't think the llvm bit is affected).

@matthiaskoenig
Copy link
Collaborator Author

Thanks. Everything works. Thanks for the fast bugfix.

@hsauro
Copy link

hsauro commented Nov 12, 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

No branches or pull requests

3 participants