fix: report ftINIT gap-fill failures instead of reporting success

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

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);

INIT/ftINITFillGapsMILP.m

@@ -231,14 +231,20 @@
 
 % Optimize the problem
 res = optimizeProb(prob,params,verbose);
-isFeasible=checkSolution(res);
+[isFeasible, isOptimal]=checkSolution(res);
 
 if ~isFeasible
     x=[];
     I=[];
     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!

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();