Update maudtools to work with latest version of Maud (now with nicer sbml) #20
Conversation
Hani thinks these might cause Matlab errors
| import os | ||
|
|
||
| import pandas as pd | ||
| import importlib_resources |
There was a problem hiding this comment.
For a future PR, we may want to add integration testing with the click testing capabilities so that we verify that cli.py is still working.
| from maud.data_model.maud_input import MaudInput | ||
|
|
||
|
|
||
| def generate_inits( | ||
| idata: az.InferenceData, mi: MaudInput, chain: int, draw: int, warmup: int | ||
| ) -> pd.DataFrame: | ||
| ) -> dict: |
There was a problem hiding this comment.
dict as a qualified keyword will make anything python <=3.8 fail, but I saw that the minimum version was bumped up to 3.9 so I guess it's fine.
There was a problem hiding this comment.
I didn't realise this but I think that is good - better to fail as much as possible in the bad python case
| @@ -3,8 +3,8 @@ | |||
| import arviz as az | |||
There was a problem hiding this comment.
I have yet to test this functionality. I am sampling a model with the new format, so I'll give a check once it finishes (same with the generate_sbml part).
There was a problem hiding this comment.
UPDATE: still works fine. The problem now is that retrieving yconc and fluxes is more complicated since it is not shaped into a experiment, metabolites array, but that's an issue with Maud itself (using maud-metabolic-models==0.4.0.2).
| # prepare scatter intervals | ||
| if non_negative: | ||
| df_priors.location = np.exp(df_priors.location) | ||
| df_priors.scale = np.exp(df_priors.scale) | ||
| df_priors["y_max"] = df_priors.location / df_priors.scale ** 2 | ||
| df_priors["y_min"] = df_priors.location * df_priors.scale ** 2 | ||
| df_priors["y_max"] = df_priors.location / df_priors.scale**2 |
There was a problem hiding this comment.
black prefers the spaces here, I don't mind too much.
There was a problem hiding this comment.
I think my black actually did this - looks like a relatively recent change psf/black#2726
| @@ -3,8 +3,21 @@ | |||
| This function takes in an InferenceData, MaudInput, chain, draw and experiment | |||
There was a problem hiding this comment.
I am getting the following error (maudtools generate-sbml output_dir):
Traceback (most recent call last):
File "/home/georg/.virtualenvs/maud-local/bin/maudtools", line 8, in <module>
sys.exit(cli())
File "/home/georg/.virtualenvs/maud-local/lib/python3.9/site-packages/click/core.py", line 1130, in __call__
return self.main(*args, **kwargs)
File "/home/georg/.virtualenvs/maud-local/lib/python3.9/site-packages/click/core.py", line 1055, in main
rv = self.invoke(ctx)
File "/home/georg/.virtualenvs/maud-local/lib/python3.9/site-packages/click/core.py", line 1657, in invoke
return _process_result(sub_ctx.command.invoke(sub_ctx))
File "/home/georg/.virtualenvs/maud-local/lib/python3.9/site-packages/click/core.py", line 1404, in invoke
return ctx.invoke(self.callback, **ctx.params)
File "/home/georg/.virtualenvs/maud-local/lib/python3.9/site-packages/click/core.py", line 760, in invoke
return __callback(*args, **kwargs)
File "/home/georg/.virtualenvs/maud-local/lib/python3.9/site-packages/maudtools/cli.py", line 112, in generate_sbml_command
experiment.id for experiment in mi.measurements.experiments
AttributeError: 'MaudInput' object has no attribute 'measurements'
There was a problem hiding this comment.
Thanks, the sbml now works on my computer
There was a problem hiding this comment.
Only balanced metabolites are reported as Species when I run it, generating in the end empty reactions (empty meaning without metabolites, although they have kinetic laws). Is that intended?
There was a problem hiding this comment.
I think this is intended - the unbalanced metabolites appear in Maud's Stan model as a parameter array called conc_unbalanced so I thought they should be represented in sbml in the same way as other parameters.
There was a problem hiding this comment.
Okay, I see.
Another way to do it is to add unbalanced metabolites as any other Species with the attribute boundary_condition set to "true". This way the model can be parsed and inspected in a normal way, but it may be ambiguous if the parsing is not careful (which is a concern, I believe).
If you think it is fine as it is I can approve the PR, I don't have any more comments.
There was a problem hiding this comment.
I'd quite like to get this change merged soon but I've made an issue to look into the best way to do this separately.
This change updates maudtools to keep up with the latest Maud (0.4.0.2).
It also improves the sbml export a lot, so that a) it only depends on the official libsbml library and b) the sbml file contains a lot more information.
Since Maud now includes example data in its package it was possible to delete a lot of duplicate test data.