fix the objective in CycleFreeFlux

[cdiener]

• [X] fix #1262
• [X] fix #935
• [X] description of feature/fix
• [X] tests added/passed
• [X] add an entry to the next release
• [X] updated docs

A bit embarrassing I took so long for such a simple fix, but here it is.

This now fixes the objective value as well when running CycleFreeFlux as in the original publication.

Fixed behavior

Compare with #1262:

Python 3.11.4 (main, Jun  7 2023, 10:13:09) [GCC 12.2.0]
Type 'copyright', 'credits' or 'license' for more information
IPython 8.8.0 -- An enhanced Interactive Python. Type '?' for help.

In [1]: from cobra.io import load_model
   ...: from cobra.flux_analysis.loopless import add_loopless, loopless_solution
   ...: 
   ...: salmonella = load_model('salmonella')
   ...: 
   ...: salmonella.objective = "GLCptspp"
   ...: 
   ...: for i in range(8):
   ...:     print(f"Loopless FBA {i}: {loopless_solution(salmonella).fluxes['GLCptspp']}")
   ...: 
Loopless FBA 0: 5.0
Loopless FBA 1: 5.0
Loopless FBA 2: 5.0
Loopless FBA 3: 5.0
Loopless FBA 4: 5.0
Loopless FBA 5: 5.0
Loopless FBA 6: 5.0
Loopless FBA 7: 5.0

[cdiener]

Tests will probably fail for now until #1340 is merged and this has been rebased.

EDIT: not necessary since CI does not check with symengine yet

[codecov-commenter]

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.09% :warning:

Comparison is base (c6f0bb4) 83.93% compared to head (aaa5943) 83.85%. Report is 5 commits behind head on devel.

:exclamation: Current head aaa5943 differs from pull request most recent head 9083275. Consider uploading reports for the commit 9083275 to get more accurate results

Additional details and impacted files

```diff @@ Coverage Diff @@ ## devel #1341 +/- ## ========================================== - Coverage 83.93% 83.85% -0.09% ========================================== Files 66 66 Lines 5497 5499 +2 Branches 1257 1257 ========================================== - Hits 4614 4611 -3 - Misses 567 572 +5 Partials 316 316 ``` | [Files Changed](https://app.codecov.io/gh/opencobra/cobrapy/pull/1341?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=opencobra) | Coverage Δ | | |---|---|---| | [src/cobra/flux\_analysis/loopless.py](https://app.codecov.io/gh/opencobra/cobrapy/pull/1341?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=opencobra#diff-c3JjL2NvYnJhL2ZsdXhfYW5hbHlzaXMvbG9vcGxlc3MucHk=) | `86.81% <100.00%> (+0.29%)` | :arrow_up: | ... and [1 file with indirect coverage changes](https://app.codecov.io/gh/opencobra/cobrapy/pull/1341/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=opencobra)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[cdiener]

Some minor comments. Are you sure that the original intention wasn't that if the objective was part of a cycle, breaking that cycle could lead to a lower optimum value? It seems to me that this will no longer be possible now.

Yes that was my intention when I originally wrote that code. But as others have pointed out now that actually does not work (https://github.com/opencobra/cobrapy/issues/698, https://github.com/opencobra/cobrapy/issues/1262) and can lead to wrong results. So that's why this now returns it to the exact implementation from the CycleFreeFlux paper (which did fix the objective). If you wanted to do this you would have to run a strategy similar to the loopless FVA where that is done with an iterative approach. We could add something like this if we wanted it in another PR.

[Midnighter]

Cheers, thank you for the work and the explanation 😃