From d238bf47dd5f93737d8f5e972b4fde5642fbd21b Mon Sep 17 00:00:00 2001 From: Eduard Kerkhoven Date: Fri, 17 Jul 2026 12:56:14 +0200 Subject: [PATCH] fix: report ftINIT gap-fill failures instead of reporting success MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- INIT/ftINIT.m | 11 ++++++++++- INIT/ftINITFillGapsForAllTasks.m | 23 ++++++++++++++++++++--- INIT/ftINITFillGapsMILP.m | 8 +++++++- testing/function_tests/tINIT.m | 21 +++++++++++++++++++++ 4 files changed, 58 insertions(+), 5 deletions(-) diff --git a/INIT/ftINIT.m b/INIT/ftINIT.m index 8ea1a059..b242346d 100644 --- a/INIT/ftINIT.m +++ b/INIT/ftINIT.m @@ -229,6 +229,7 @@ first = true; success = false; fullMipRes = []; + lastError = []; for rn = 1:length(stp.MILPParams) params = stp.MILPParams{rn}; if ~isfield(params, 'MIPGap') @@ -275,13 +276,21 @@ catch e mipGap = Inf; lastObjVal = Inf; %we need to set something here, Inf leads to that this doesn't come into play + lastError = e; %kept so a throw is not reported as a time-limit miss end success = mipGap <= params.MIPGap; end if ~success - error('RAVEN:badInput', '%s', ['Failed to find good enough solution within the time frame. MIPGap: ' num2str(mipGap)]); + if ~isempty(lastError) + %The MILP threw rather than returning a poor gap; reporting this + %as a time-limit miss sends users off to raise params.TimeLimit + %for what is often a solver or input problem. + error('RAVEN:badInput', '%s', ['Failed to find good enough solution within the time frame. The last MILP attempt failed with: ' lastError.message]); + else + error('RAVEN:badInput', '%s', ['Failed to find good enough solution within the time frame. MIPGap: ' num2str(mipGap)]); + end end %save the reactions turned on and their fluxes for the next step diff --git a/INIT/ftINITFillGapsForAllTasks.m b/INIT/ftINITFillGapsForAllTasks.m index 8e521b5c..e030a320 100644 --- a/INIT/ftINITFillGapsForAllTasks.m +++ b/INIT/ftINITFillGapsForAllTasks.m @@ -1,4 +1,4 @@ -function [outModel, addedRxns]=ftINITFillGapsForAllTasks(model,refModel,inputFile,printOutput,rxnScores,taskStructure,params,verbose) +function [outModel, addedRxns, failedTasks]=ftINITFillGapsForAllTasks(model,refModel,inputFile,printOutput,rxnScores,taskStructure,params,verbose) % ftINITFillGapsForAllTasks % Fills gaps in a model by including reactions from a reference model, % so that the resulting model can perform all the tasks in a task list. @@ -28,6 +28,11 @@ % for each task (N). An element is true if the corresponding % reaction is added in the corresponding task. % Failed tasks and SHOULD FAIL tasks are ignored +% failedTasks Nx1 logical, true for each task that could not be +% gap-filled, either because no feasible solution exists +% using the reference model or because the attempt threw. +% Such tasks add no reactions, so they are otherwise +% indistinguishable from tasks that already worked % % This function fills gaps in a model by using a reference model, so % that the resulting model can perform a list of metabolic tasks. The @@ -74,6 +79,7 @@ tModel=model; addedRxns=false(numel(refModel.rxns),numel(taskStructure)); +failedTasks=false(numel(taskStructure),1); supressWarnings=false; nAdded=0; @@ -308,15 +314,26 @@ failed=false; try [newRxns, newModel, exitFlag]=ftINITFillGaps(tModel,model,tRefModel,false,supressWarnings,tRxnScores,params,verbose); - if exitFlag==-2 + if exitFlag==-1 + %No set of reactions from the reference model makes the + %task feasible. Without this branch the task falls + %through and reports "Added 0 reaction(s)", which is the + %same thing an already-feasible task reports. + EM=['"[' taskStructure(i).id '] ' taskStructure(i).description '" could not be gap-filled: no feasible solution exists using the reference model\n']; + warning('RAVEN:warning', '%s', EM); + failed=true; + elseif exitFlag==-2 EM=['"[' taskStructure(i).id '] ' taskStructure(i).description '" was aborted before reaching optimality. Consider increasing params.maxTime\n']; warning('RAVEN:warning', '%s', EM); end catch e - EM=['"[' taskStructure(i).id '] ' taskStructure(i).description '" could not be performed for any set of reactions\n']; + EM=['"[' taskStructure(i).id '] ' taskStructure(i).description '" could not be performed for any set of reactions: ' e.message '\n']; warning('RAVEN:warning', '%s', EM); failed=true; end + if failed + failedTasks(i)=true; + end if failed==false if ~isempty(newRxns) nAdded=nAdded+numel(newRxns); diff --git a/INIT/ftINITFillGapsMILP.m b/INIT/ftINITFillGapsMILP.m index 7e057258..7c13dfa4 100644 --- a/INIT/ftINITFillGapsMILP.m +++ b/INIT/ftINITFillGapsMILP.m @@ -231,7 +231,7 @@ % Optimize the problem res = optimizeProb(prob,params,verbose); -isFeasible=checkSolution(res); +[isFeasible, isOptimal]=checkSolution(res); if ~isFeasible x=[]; @@ -239,6 +239,12 @@ exitFlag=-1; return; end +if ~isOptimal + %A feasible but suboptimal solution, i.e. the solver stopped on its time + %limit. The solution is still returned, but must not be reported as + %optimal: the gap-fill it describes may be far from the smallest one. + exitFlag=-2; +end x=res.full(1:numel(model.rxns));%the fluxes I=res.full(intsIndexes) > 10^-3;%The margin for integers in gurobi is 10^-5, not 10^-12 that was previously used! Use 10^-3 to have some margin! diff --git a/testing/function_tests/tINIT.m b/testing/function_tests/tINIT.m index 5c77724c..f82a4484 100644 --- a/testing/function_tests/tINIT.m +++ b/testing/function_tests/tINIT.m @@ -172,6 +172,27 @@ function ftINITFillGapsForAllTasksRuns(testCase) testCase.verifyTrue(all(strcmp(mTempRef.rxns(addedRxnMat), 'R7'))); end + function ftINITFillGapsForAllTasksReportsUnfillableTask(testCase) + % R7 is required for the task, so removing it from the reference + % model as well leaves the task unfillable. That must be reported, + % not reported as "Added 0 reaction(s)" like an already-working + % task. + testCase.assumeMILPSolver(); + testModel = getTstModel(); + testModelTasks = getTstModelTasks(); + testRxnScores = getTstModelRxnScores(); + + mTempRef = closeModel(testModel); + mTempRef = removeReactions(mTempRef, {'R1';'R7';'R8'}); + mTemp = mTempRef; + mTemp.id = 'tmp'; + tmpRxnScores = testRxnScores([2;3;4;5;6;9;10]); + evalc(['[~,addedRxnMat,failedTasks] = ftINITFillGapsForAllTasks(mTemp,' ... + 'mTempRef,[],false,min(tmpRxnScores,-0.1),testModelTasks);']); + testCase.verifyTrue(failedTasks(1)); + testCase.verifyFalse(any(addedRxnMat(:))); + end + function ftINITMetabolomicsRuns(testCase) % Detected metabolites steer ftINIT towards alternative pathways. testCase.assumeMILPSolver();