fix: isozyme kcat selection and EC-annotation overwrite bugs - #463
Merged
Conversation
…igned isozymes' fabricated zero cost In the light formulation, an isozyme with no kcat assigned (kcat==0, giving an Inf MW/kcat) had its cost corrected to zero before taking min() across a reaction's isozymes, so that fabricated zero always won over any real kcat on another isozyme of the same reaction -- silently leaving the reaction under-constrained by enzyme usage. Isozymes with no usable kcat (or unresolved MW) are now dropped before the min() instead, matching geckopy's apply_kcat_constraints; a reaction with no valid isozyme at all is still left uncosted, as before. raven-gecko-parity#57
…ty sources overwrite=true copied ecModel.ec.eccodes verbatim, including empty entries, so an existing populated ecModel.eccodes annotation could be silently erased just because the corresponding ec-structure entry happened to be empty. An empty source entry is now never copied, whether or not overwrite is set, matching geckopy's copy_ec_to_gem (which already treats an empty ec.eccodes entry as "no info to propagate"). raven-gecko-parity#79
edkerk
force-pushed
the
fix/isozyme-kcat-and-ec-overwrite
branch
from
August 30, 2026 22:00
af098ef to
870c96b
Compare
Unit test results50 tests 50 ✅ 10s ⏱️ Results for commit e440d08. |
This was referenced Aug 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two independent correctness fixes, both confirmed against geckopy's already-correct behavior and decided in raven-gecko-parity#57 / raven-gecko-parity#79:
applyKcatConstraints(light formulation) — when a reaction had both a kcat-assigned isozyme and an unassigned one, the unassigned isozyme'sInfcost (kcat==0) was corrected to0beforemin()was taken across isozymes, so that fabricated zero always won over any real kcat — silently leaving the reaction under-constrained by enzyme usage, with no warning. Isozymes with no usable kcat (or unresolved MW) are now excluded before the comparison instead, matching geckopy'sapply_kcat_constraints. A reaction with no valid isozyme at all is still left uncosted, same as before.copyECtoGEM—overwrite=truecopiedec.eccodesverbatim, including empty entries, so a real, populatedecModel.eccodesannotation could be silently erased just because the correspondingec-structure entry happened to be empty. An empty source entry is now never copied, whether or notoverwriteis set — matching geckopy'scopy_ec_to_gem, which already documents this exact MATLAB behavior as a divergence it deliberately didn't replicate.Both fixes and their four new regression tests (
geckoCoreFunctionTests.mtc0047–tc0050) were reviewed with @eduardk before implementation — see raven-gecko-parity#57 and raven-gecko-parity#79 for the discussion.Fixes raven-gecko-parity#57
Fixes raven-gecko-parity#79
Test plan
geckoCoreFunctionTests: 50 passed, 0 failed (full suite, RAVEN develop3 on path, glpk solver)overwrite=trueno longer erases a populated annotation with an empty source;overwrite=falsestill only fills empty entries