fix: report ftINIT gap-fill failures instead of reporting success - #678
Merged
Conversation
Three ways a failed gap-fill was reported as a success: ftINITFillGapsMILP discarded checkSolution's isOptimal, so a solve that hit its time limit returned exitFlag=1 and its suboptimal gap-fill was reported as optimal. That also left the documented -2 (time out) unreachable, which in turn made the exitFlag==-2 branch in ftINITFillGapsForAllTasks dead code. ftINITFillGapsForAllTasks never checked for exitFlag==-1, so a task no set of reference reactions can fulfil fell through and printed "Added 0 reaction(s)" — the same line an already-feasible task prints. It now warns and records the task in a new failedTasks output. Both catch blocks discarded the exception. ftINITFillGapsForAllTasks now includes e.message in its warning, and ftINIT no longer reports a throw as "Failed to find good enough solution within the time frame. MIPGap: Inf", which sends users to raise TimeLimit for what is usually a solver or input problem. Adds a test for an unfillable task.
Function test results241 tests 218 ✅ 57s ⏱️ Results for commit d238bf4. |
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.
Main improvements in this PR:
ftINITFillGapsForAllTasksreported a task that cannot be gap-filled asAdded 0 reaction(s), the same line an already-feasible task prints. It now warns and returns which tasks failed.ftINITFillGapsMILPreported a solve that hit its time limit as optimal, becausecheckSolution's optimality flag was discarded. This also left the documented time-out exit code unreachable.ftINITreported an exception from the MILP asMIPGap: Inf, i.e. as a time-limit miss, hiding the real error. Both catch blocks now surface the message.ftINITFillGapsForAllTasksreturnsfailedTasks.tINIT.m.Instructions on merging this PR:
develop3as target branch, and will be resolved with a squash-merge.