cobratoolbox icon indicating copy to clipboard operation
cobratoolbox copied to clipboard

RelaxedFBA tutorial bugs

Open shjchan opened this issue 8 years ago • 4 comments

@fromseto and I have tried to run the latest version (as of July 12, 2017) of tutorial_relaxedFBA.mlx from the develop branch. We found that:

  1. The .mlx and .m files do not read the same (.mlx is more recently updated).
  2. In the .mlx file, there are bugs in the first code block loading the 'Recon2.0model':
filename2='Recon2.0model';
if exist('Recon2.0model.mat','file')==2
      model = readCbModel(fileName);
end

fileName, filename2 do not match and 'readCbModel' is unable to handle filename with '.' inside. This can be fixed easily by using load instead.

  1. But the default setting in the tutorial for Recon2.0 model results in an error in both of our trials. The code leading to the error in the tutorial is:
relaxOption.internalRelax = 0;
relaxOption.exchangeRelax = 2;
relaxOption.steadyStateRelax = 0;
feasTol = getCobraSolverParams('LP', 'feasTol');
relaxOption.epsilon = feasTol/100;%*100;
relaxOption.gamma  = 10;
relaxOption.lambda = 10;  
tic;
solution = relaxedFBA(model,relaxOption);

Then the following error occurred:

Warning: solveCobraLP solution status is 0 
> In relaxFBA_cappedL1>relaxFBA_cappedL1_solveSubProblem (line 348)
  In relaxFBA_cappedL1 (line 170)
  In relaxedFBA (line 211) 
Error using relaxFBA_cappedL1 (line 179)
Problem infeasible !
Error in relaxedFBA (line 211)
solution = relaxFBA_cappedL1(model,relaxOption);

But if we set relaxOption.internalRelax = 2 , then relaxFBA finishes without error. We are not sure whether it needs to be fixed, or it is the problem of the particular model.

I hereby confirm that I have:

  • [X] Tried to solve the issue on my own
  • [X] Retried to run my code with the latest version of The COBRA Toolbox
  • [X] Checked that a similar issue has not already been opened

(Note: You may replace [] with [X] to check the box)

shjchan avatar Jul 12 '17 22:07 shjchan

Indeed, relaxedFBA doesn´t handle lb < 1 which was the case for Recon2 bounds, this is now fixed in #786 . A workaround would have been to multiply the bounds by 1e9. Thanks for reporting!

marouenbg avatar Jul 17 '17 21:07 marouenbg

@shjchan, thanks for reporting this.

PR#786 addresses 1). Thanks @marouenbg! 👍

In regards to 2), the lines should read:

fileName = 'Recon2.0model.mat';
if exist(fileName, 'file') == 2
      model = readCbModel(fileName);
end

The issue is not that readCbModel cannot read files with . in the name. The issue here is that there is no valid file extension. This works perfectly fine:

>> readCbModel('Recon2.0model.mat')

@marouenbg, may you make that change in PR #786? Thanks.

laurentheirendt avatar Jul 19 '17 13:07 laurentheirendt

it was changed to load instead of readCbModel like suggested up, and 3) is fixed as well

marouenbg avatar Jul 19 '17 13:07 marouenbg

The recommendation is to use readCbModel() instead of load(). The reason is that readCbModel also checks the model and adds missing fields, whereas load() does not.

laurentheirendt avatar Jul 19 '17 14:07 laurentheirendt