-
Notifications
You must be signed in to change notification settings - Fork 53
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
feat: align RAVEN and COBRA model fields #184
Comments
Hi,
|
Oh and one more thing: |
|
Regarding For instance, in GECKO we first recreate |
@BenjaSanchez in that case, you also don't use the model-provided I suppose the question is whether you perform other changes on the model (add reactions etc.) while you want to retain the |
@edkerk yes, however I don't know if e.g. leaving exchange rxns with |
@edkerk wrt to @BenjaSanchez I agree that people might want to define rev, but historically, it was used as a model reversibility indicator and, unfortunately is present in old model mat files in exactly this fashion. |
@tpfau, |
@simas232 The 'whats important or not' question is one reason why we don't have a annotation struct. Its simply up to the user what she/he thinks is important and we don't take the decision. For anything we don't know, we will write a kind of descriptive field name, but things we do know will get parsed into specific fields (a list which we are happy to extend, but which needs no changes in the code). An additional argument (not mine) for having the individual fields in our model structure was the aim of a "flat" data structure, i.e. no substructures, and admittedly I would not expect a cell array of (similar) structs but rather a struct array for something like function model = removeCompartment(model,compartment);
rxnsToRemove = findRxnsFromMets(model,model.mets(strcmp(model.metComps,compartment)));
model = removeFieldEntriesForType(model,strcmp(model.comps,compartment));
model = removeRxns(model,rxnsToRemove);
model = removeUnusedGenes(model); While with metComps being a double array we would need: function model = removeCompartment(model,compartment);
compPos = find(strcmp(compartment,model.comps));
model = removeFieldEntriesForType(model,strcmp(model.comps,compartment));
rxnsToRemove = findRxnsFromMets(model,model.mets(model.metComps == compPos));
model = removeRxns(model,rxnsToRemove);
model = removeUnusedGenes(model);
% code to adjust the model.metComps field.
posToChange = model.metComps > compPos;
model.metComps(posToChange) = model.metComps(posToChange) - 1 Admittedly this is not too bad but it is something that might be necessary again and again in functions that manipulate compartments, e.g. when merging two models or similar things. |
@tpfau, thank you for a comprehensive explanation. I reckon it is okay to abolish numeric @edkerk, @Hao-Chalmers, @BenjaSanchez, @IVANDOMENZAIN, @JonathanRob, @danieljcook, do you foresee any potential problems if we discard |
I wasn't aware of that field, as I haven't seen it used yet. But well, it doesn't hurt too much (its not very big), to have such a field.
Thanks, that was quite some work :)
We actually do check IDs for defined annotation fields. i.e. were miriam has a spec, we try to use valid IDs. Only for fields which we have not explicitly encoded, this step is skipped. |
@simas232 regarding @tpfau would you then advocate for only using Moving towards a cell array of strings for Regarding |
The multiples values could be separated by semicolon, which is used in some fields (e.g. |
Yes, and no.
Yes, that would be the idea.
Currently (we could change this), we store muliple IDs by separating them with |
@edkerk |
Thats perfectly fine with me. |
My idea is to make it optional, so just leave it out if no version is provided in the model.
No clue about this, because SBML spec is out of our hands. In the beginning, we probably just implement this into Matlab model structure when dealing with repo-type GEMs, and push this forward into SBML spec if it turns out a good idea. |
There are many benefits in having RAVEN functioning as submodule of the COBRA toolbox. While RAVEN 2.0 is largely compatible with COBRA through use of the
ravenCobraWrapper
function and using unique function names, a complete alignment of model structure between the two toolboxes would be highly preferred.To initiate this process, here are the definitions of the RAVEN fields.
The main discrepancies are:
metComps
field, instead of detailing this in metabolite IDsmetMiriams
and similar fields, instead ofmetKEGGID
and similar fieldsHow should these fields be unified? There is interest from COBRA to move towards the use of the
metComps
field, but what about other discrepancies? What other discrepancies exist?The text was updated successfully, but these errors were encountered: