-
Notifications
You must be signed in to change notification settings - Fork 217
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
design / implement better management of growth media #303
Comments
Uh nice. I have two things close to my heart ;)
|
One of the ways I've been thinking about implementing this was overwriting Model.media to be a property where we could set the getters and setters. In the get method, I would imagine calling a function we write in cobra.manipulation which looks for boundary reactions that are not knocked out. We'll have to make some decisions on directionality... for instance, if a model has a reaction In the set method, I'd imagine just the reverse of the getter. Iterate over the dict-like object, maybe something like {'EX_glc_e': (0, 1000),
'EX_h_e': (-1000, 1000),
} and then set the bounds for each reaction as appropriate. In terms of storing media along with the model, I would think the best place to do this would be in the model.notes dictionary. Just have a key for model.meda = 'glucose'
model.optimize()
model.media = 'xylose'
model.optimize() would work, if {'glucose': {'EX_glc_e': ...},
'xylose': {'EX_xyl_e': ...}}
|
|
A few more considerations:
|
Lot's of good points here.
There is already a with model.medium("LB"):
model.optimize()
with model.knockout("EX_glc_e").medium("SB"): # that would be even cooler
model.optimize() |
Agreed that compartment handling might want to be its own package. I just don't see a clear path towards making that general enough for all cobra models. On context managers, I'm 100% on board. One proposed solution was the I wonder if we could make cobra models copy-on-write... Then we could spin up new models whenever we want without too much concern. i.e., Otherwise we could define context manager functions like you're describing, where we implement something similar to TimeMachine under the hood. ( My point with CO2 was that if the media also needs to include bounds on secreted species, we'll need a term for every boundary species in every media. Unless we define a 'default' media, and subsequent media are just diffs from that? Might be tough to keep everything straight, I'd want the default to be tied to the bounds as defined when the model was loaded / saved. |
Boundary metabolites can easily lead to bugs. They really don't make sense They are a big part of this issue: On Oct 28, 2016 9:04 AM, "Peter St. John" notifications@github.com wrote:
|
Great point, I'm fine with leaving it as reactions then |
I think nobody want to use boundary metabolites. Cobra does not read them and should not. The question is whether we want a helper function to identify the media reactions based on a list of metabolites. Otherwise if I have a culture medium from the literature I would have to:
I think at least step 2 should have an option to be automatized, otherwise picking reaction out of a list of several thousand would be large source of error and bring us back to the reproducibility problem. So I am lobbying for a: model.media = medium_from_metabolites(metabs, bound=1000) or something similar. Your for r in many_reactions: knock_out(model, r).optimize() I think I also prefer the if the context managers would be methods of the Model class since that would allow pandas style piping, e.g. medium(knock_out(knock_out(model, "r1"), "r2"), "LB").optimize() is uglier than model.knock_out("r1").knock_out("r2").medium("LB").optimize() IMHO. However, that would not work with context managers... |
exactly, we couldn't rely on the existing copy machinery. I was thinking something along the lines of what was done in this library: https://github.com/diffoperator/pycow, but I dont know how well that would work with the nested structures that build up into a model. I.e., the copied (proxy) model would have to have its reactions point back to the base, but then when somethings changed it would have to spin up a new reaction. Or a proxy of the original reaction. Anyways, it would be a cool way of forking models without much memory or computational overhead, which I think is the root of the issue we're looking at. For enter and exit methods, I agree on avoiding the nested functional calls. An alternative could be to just define enter / exit methods on the model itself? I'm not 100% clear on if this would work.. with model:
model.medium = 'glucose'
model.reactions.my_favorite_reaction.knock_out()
stored_solution_somewhere = model.optimize() where |
Exactly, something like pycow would be necessary to manage that kind of context manager. Maybe a "dumb" TimeMachine aka from contextlib import contextmanager
@contextmanager
def tm(model):
yield model.copy()
with tm(model) as m:
m.reactions.mfr.knock_out()
m.medium = "glucose" And add only some specific ones for media and multiple knockouts as with model.medium("glucose"):
model.bla_bla()
with model.knock_out(("r1", "r2", ...)):
model.bla_bla() |
Hi, @hredestig, do you think it would be good idea to open a feature branch for that and add @pstjohn and @mmundy42 with write access to the branch so we can play around with some options? |
Just add @mmundy42 to cobrapy core. @cdiener, on the time-machine methods, I would guess that you could initialize some type of history-tracker and attach it to self on Are reaction bounds the only thing we're tracking? If so it could be as easy as just doing an On more careful thought the copy-on-write would be difficult to implement with the current organization of Models, Reactions, and Metabolites. The reason being is that not only to objects point down the hierarchy: Model -> Reaction -> Metabolite, but they also point up: metabolite.model keeps track of what model its a part of, and same with Reactions. If we switched to a top-down only model, implemented perhaps with something like networkx, then we could do really fast shallow copies of a model's graph when calling model.copy. The problem here though is that you'd loose the ability to use methods like metabolite.reactions, since metabolites wouldn't know which reactions they're a part of, and reactions wouldn't know what models they're a part of. You could get around this partly by passing models as arguments, and partly by implementing methods at the next level up. (model.get_reactions(metabolite_id)) This is obviously a much bigger restructuring than just media... Has anyone thought about this type of organization flow? |
@pstjohn I think in order to make the behavior understandable we would need to track all attributes. This would also include the solver references that will get introduced by optlang. It seems that this feature would generare a lot of boiler plate code and would also break everytime you add new attributes to |
Very much agree that media should somewhere be a list of metabolites as that is what it actually is and media reactions something computed from the current model and this list of media constituents.. Could we possibly create a different issue with the purpose of discussing context managers, propose different alternatives for reversible changes to the model as it is not closely related to the media discussion? While the time-machine might not be perfect in all regards (also like method chaining and operating primarily on the model..), however, for refactoring methods from cameo to cobra there is quite some effort to be saved with at least having a compatible implementation since cameo makes heavy use of that class. On new branch, indeed, you can just open branches as you like and @mmundy42 would you please request to join https://github.com/orgs/opencobra/teams/cobrapy-core if you like to contribute directly. |
@hredestig, when I try the link I get a 404 error from github.com. Is that the right link or am I doing something wrong? Maybe I need to be invited to the organization first? https://help.github.com/articles/inviting-users-to-join-your-organization/ |
Closing with #350, lets open a new one for further discussions |
I'd like to start a discussion on how to implement management of growth media. I can think of two approaches:
In either approach exchange reactions that are not in the input would have the bounds set to 0.
I also think there should be a way to save media to a file so they can be shared with others.
Thoughts? Other ideas?
The text was updated successfully, but these errors were encountered: