Skip to content

Make FBA LP backend solver exchangeable (#71) - #99

Closed
dyrpsf wants to merge 3 commits into
draeger-lab:masterfrom
dyrpsf:exchangeable-solvers
Closed

Make FBA LP backend solver exchangeable (#71)#99
dyrpsf wants to merge 3 commits into
draeger-lab:masterfrom
dyrpsf:exchangeable-solvers

Conversation

@dyrpsf

@dyrpsf dyrpsf commented Jan 8, 2026

Copy link
Copy Markdown
Contributor

This PR implements the “exchangeable solvers” feature for Flux Balance Analysis (issue #71).

What this PR does

  • Adds an overloaded constructor

    FluxBalanceAnalysis(SBMLDocument doc, LinearProgramSolver solver)

    so that any SCPSolver LinearProgramSolver implementation can be used as the backend
    for FBA.

  • Keeps the existing constructor

    FluxBalanceAnalysis(SBMLDocument doc)

    but makes it delegate to the new one with new NewGLPKSolver(), so GLPK remains the
    default backend and existing code continues to work unchanged.

  • Updates solve() to call the injected solver field instead of a hard‑coded GLPK
    instance.

Note about Commons Math

This branch previously also contained the Commons Math 3 dependency change that belongs
to PR #98 (issue #92). That change has now been reverted on this branch
(commit Revert "Add Commons Math 3 dependency alongside Commons Math 2.x (#92)"),
so the net effect of this PR is only the FBA solver change.

The Commons Math 3 update itself remains confined to PR #98, which is currently on hold
as requested while the OptSolvX work is completed.

@dyrpsf dyrpsf mentioned this pull request Jan 8, 2026
@dyrpsf

dyrpsf commented Jan 8, 2026

Copy link
Copy Markdown
Contributor Author

I just noticed that this branch accidentally includes the Commons Math 3 dependency commit that
already belongs to PR (#98).

To clarify:

Sorry for the confusion. If you would prefer a completely clean PR that only contains the solver
change (without the Commons Math commit), I’m happy to recreate it on a new branch based on
upstream/master.

@xts-Michi
xts-Michi self-requested a review January 8, 2026 11:55
@dyrpsf

dyrpsf commented Jan 8, 2026

Copy link
Copy Markdown
Contributor Author

Quick update: I’ve now reverted the Commons Math 3 dependency commit from this branch, so
the effective changes in this PR are only the FBA solver refactoring in FluxBalanceAnalysis.

The Commons Math 3 update is still present only in PR #98 (for issue #92), which will stay
on hold as requested while the OptSolvX backend is being completed. This PR (#99) can now
be reviewed independently, as it no longer changes pom.xml.

@xts-Michi

Copy link
Copy Markdown
Collaborator

Thanks for the PR.
Note that SBSCL's FBA backend is being refactored to use OptSolvX (SCPSolver-free), with backend selection via optsolvx.solver.
Your changes make the backend pluggable via SCPSolver's LinearProgramSolver and retain GLPK as the default. This overlaps conceptually with issue #71, but uses a different solver abstraction and would effectively reintroduce SCPSolver as the extension API.

Because we want multiple interchangeable solver backends, we introduced OptSolvX, which is in the final development phase of integration into SBSCL.
Therefore, this PR is unfortunately redundant and will be closed.

@xts-Michi xts-Michi closed this Jan 8, 2026
@dyrpsf

dyrpsf commented Jan 9, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the PR. Note that SBSCL's FBA backend is being refactored to use OptSolvX (SCPSolver-free), with backend selection via optsolvx.solver. Your changes make the backend pluggable via SCPSolver's LinearProgramSolver and retain GLPK as the default. This overlaps conceptually with issue #71, but uses a different solver abstraction and would effectively reintroduce SCPSolver as the extension API.

Because we want multiple interchangeable solver backends, we introduced OptSolvX, which is in the final development phase of integration into SBSCL. Therefore, this PR is unfortunately redundant and will be closed.

Thanks for the detailed explanation and for taking the time to review this.

It makes sense to keep the FBA backend fully aligned with the new OptSolvX-based design and to
avoid reintroducing SCPSolver as another abstraction layer. I’m happy for this PR to be closed
as redundant given that OptSolvX will provide the multiple interchangeable solver backends.

I’ll read through the OptSolvX integration work (PR #90) and would be glad to help with any
follow‑up tasks or smaller issues that come up around the new backend.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants