cobratoolbox icon indicating copy to clipboard operation
cobratoolbox copied to clipboard

optimizeCbModel return values

Open tpfau opened this issue 6 years ago • 3 comments

Currently, optimizeCbModel returns a solution with plenty of fields which are redundant e.g. v, x, full for the variable value f, obj for the objective value

Which are partially undocumented and mainly there for backward compatability reasons (e.g. v and x).

I would suggest to change this behaviour and at least remove those fields specific to solveCobraXYZ from the solution (obj, full, dual, slack, rcost), and trimming all other values to the fields of the model (including e.g. basis which is currently not trimmed). We would have to check existing code for the use of those fields, and it would be a breaking change so some old code won't work any more with this version, but could easily be adapted (or you could simply use an old revision of the toolbox if you want to run that code).

Any opinions on this?

I hereby confirm that I have:

  • [X] Checked that a similar issue has not already been opened

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

tpfau avatar Nov 20 '18 05:11 tpfau

@tpfau we have enough issues with backward compatibility at the moment.

rmtfleming avatar Nov 20 '18 11:11 rmtfleming

I know, but at some point carrying extremely old things along becomes a burden. In particular since any old script would still work with the cobra version tagged 3.0, and it would just need to be updated if it relies on these old fields and the user wants to use it with a newer version.

That is one of the reasons why we have version tags now. If your code works with a specific tagged version, it will also work with that version in the future. Its the same as changing Matlab versions. We only actively support the latest three b releases of matlab. So if you have really old code, chances are high that it won't work as intended back than anyways.

tpfau avatar Nov 30 '18 13:11 tpfau

Thanks for reporting this @tpfau. Let's put this on hold for the moment. 👍

laurentheirendt avatar Nov 30 '18 15:11 laurentheirendt