opencobra / cobrapy

COBRApy is a package for constraint-based modeling of metabolic networks.
http://opencobra.github.io/cobrapy/
GNU General Public License v2.0
467 stars 218 forks source link

Error assigning the bounds in _add_cycle_free #1374

Closed alexpan00 closed 9 months ago

alexpan00 commented 9 months ago

Is there an existing issue for this?

Problem description

In the CycleFreeFlux paper, the bounds of each reaction are forced to be 0, and the initial optimization result.

image

However, in the _add_cycle_free implementation, when the original flux is > 0, the ub is forced to be the maximum value between the flux and the reaction ub, so this will always be the reaction ub. The same happens with the lb when the original flux is negative.

        if flux >= 0:
            rxn.bounds = max(0, rxn.lower_bound), max(flux, rxn.upper_bound)
            objective_vars.append(rxn.forward_variable)
        else:
            rxn.bounds = min(flux, rxn.lower_bound), min(0, rxn.upper_bound)
            objective_vars.append(rxn.reverse_variable)

Is there any reason for this way of assigning the bounds or is this a bug?

Environment

### Package Information | Package | Version | |:--------|--------:| | cobra | 0.29.0 | ### Dependency Information | Package | Version | |:--------------------|------------:| | appdirs | 1.4.4 | | black | **missing** | | bumpversion | **missing** | | depinfo | 2.2.0 | | diskcache | 5.6.3 | | future | 0.18.3 | | httpx | 0.25.1 | | importlib-resources | 6.1.1 | | isort | **missing** | | numpy | 1.26.1 | | optlang | 1.8.1 | | pandas | 2.1.2 | | pydantic | 2.4.2 | | python-libsbml | 5.20.2 | | rich | 13.6.0 | | ruamel.yaml | 0.18.5 | | scipy | 1.11.3 | | swiglpk | 5.0.8 | | tox | **missing** | ### Build Tools Information | Package | Version | |:-----------|--------:| | pip | 23.3.1 | | setuptools | 68.2.2 | | wheel | 0.41.3 | ### Platform Information | | | |:--------|------------------------------------:| | Linux | 4.18.0-477.15.1.el8_8.x86_64-x86_64 | | CPython | 3.10.13 |
cdiener commented 9 months ago

From a cursory look it looks like you're right. Do you want to send in a PR?