make the semantics cut #776
Conversation
exaexa
added
❌ breaking
| @@ -108,7 +108,7 @@ modifying the reaction list, stoichiometry, and bounds: | |||
| COBREXA.unwrap_model(x::LeakyModel) = x.mdl | |||
| COBREXA.reaction_count(x::LeakyModel) = reaction_count(x.mdl) + 1 | |||
| COBREXA.reaction_ids(x::LeakyModel) = [reaction_ids(x.mdl); "The Leak"] | |||
| COBREXA.stoichiometry(x::LeakyModel) = [stoichiometry(x.mdl) [m in x.leaking_metabolites ? -1.0 : 0.0 for m = metabolites(x.mdl)]] | |||
| COBREXA.stoichiometry(x::LeakyModel) = [stoichiometry(x.mdl) [m in x.leaking_metabolites ? -1.0 : 0.0 for m = metabolite_ids(x.mdl)]] | |||
There was a problem hiding this comment.
probably want a better name than leaking_metabolites
| @@ -54,7 +54,7 @@ function gapfill_minimum_reactions( | |||
| precache!(model) | |||
|
|
|||
| # constraints from universal reactions that can fill gaps | |||
| univs = _universal_stoichiometry(universal_reactions, metabolites(model)) | |||
| univs = _universal_stoichiometry(universal_reactions, metabolite_ids(model)) | |||
| @@ -89,7 +89,7 @@ function gapfill_minimum_reactions( | |||
| @constraint( | |||
| opt_model, | |||
| extended_stoichiometry * [x; ux] .== | |||
| [balance(model); zeros(length(univs.new_mids))] | |||
| [metabolite_bounds(model); zeros(length(univs.new_mids))] | |||
We'd get burned at stakes if we had stoichiometry with reactions in rows.
|
@stelmo I guess this is ready for merging into the bigger PR now (at least the tests pass). It might be necessary to redo the logic of the community models a bit. Matrix passthrough still doesn't really work but that's patchable later (it's mostly a minor performance issue) |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## mk-finalize-semantics #776 +/- ##
=========================================================
- Coverage 89.03% 88.97% -0.07%
=========================================================
Files 92 92
Lines 2444 2403 -41
=========================================================
- Hits 2176 2138 -38
+ Misses 268 265 -3
☔ View full report in Codecov by Sentry. |
🚀 let's figure out the problems as they come up |
|
ok 💥 |
I have this specially separated from the rest of the (RELATIVELY HARMLESS) semantics-are-everywhere change because it's gonna be the most breaking part, and the reviews should be kinda clear.
In this state it's not looking good (tests will fail until I fix them) but most of the errors I see seem fixable.
As the main points:
stoichiometryis now just a nice name formetabolite_variables_matrixbalanceis gone, instead of that the metabolites have the balance boundsmetabolite_variablesis the master form now as with other semantics..I'll fix the rest, at this points any comments welcome.