Develop#2655
Conversation
findNonBuiltinCalls
…in getObjectiveSense and buildOptProblemFromModel
…s = true and the number of samples is lower than 5. An extensive explanation is given on why the error is thrown and how best to move forward
…ded explanations for each sheet
Back merge to Develop
minor bugfixes
…correlations, but also the associated 95% confidence intervals and p-values for each flux-taxon combination
Updated agent guidelines and preferred solver order.
model.d can represent the RHS of additional constraints
- adapt for use of arguments extraVarRelax and extraConstraintRelax
…solver swap
- size(LPProblem.A) returned a 2-vector, then csense(1:nMet,1)='E' used
it as a length, producing an incorrectly sized csense. Use
size(LPProblem.A, 1).
- Apply cpxControl.tune.{timelimit,repeat,display} only after
contFunctName has been resolved into a struct. The previous code
assigned .tune fields onto contFunctName before checking its type, so
passing a function-name string (the documented alternative) crashed.
- Permanent global side-effect: changeCobraSolver('ibm_cplex') silently
switched the user's configured LP solver during availability probing.
Restore the previous CBT_LP_SOLVER after the check.
- Pre-allocate b_L/b_U as -inf/+inf vectors of the correct length so
rows whose csense character is missing get well-defined bounds rather
than undefined trailing zeros.
- Wrap the CPLEX init failure as a typed MException with the original
exception attached via addCause (was: bare error, original cause lost).
- Replace eval-based dotted-path read/write of CPLEX params with
helpers using setfield/getfield (no eval, no silent injection risk).
- Treat empty contFunctName as struct() rather than [] so the function
always returns a struct as documented.
- Convert silent 'Optimisation failed.' fprintf into a proper warning
with an identifier (tuneParam:tuneFailed).
…er under the conditions of apptainer being accessible or not from within matlab environemt, makedb has been changed to warn on volume unavailability rather exit, apptainer definition file added to create container
Updated function documentation and comments for clarity.
Updated function documentation and improved argument handling for bio_draw_by_rxn.
Updated function documentation and improved input handling.
Updated the findNearRxns function to improve documentation and clarify input/output parameters.
Paint4Net Update - Resubmitted
Apptainer update for Persephone
Address Codex review (PR #2652): when LPProblem.csense was present but empty (e.g. ''), the bounds branch was skipped, leaving every row at [-inf, +inf] and silently tuning a free LP. Treat empty csense the same as a missing csense (default to equality, matching existing behaviour), and error out when csense length disagrees with size(A,1). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
replace inf bounds before setting param.maxUB to avoid unbounded solution
Persephone pipeline: Clarified output descriptions and new error check
fix(tuneParam): correct csense sizing, safer arg handling, no global solver swap
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a378bea221
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| modelFitted.evarlb = model.evarlb - p_e; | ||
| modelFitted.evarub = model.evarub + q_e; |
There was a problem hiding this comment.
In allConstraints mode this code unconditionally reads model.evarlb/model.evarub, but nExt can be zero when the model has only flux variables (the mode is documented as supporting flux variables and/or extra variables). In that common case, models do not define evarlb/evarub, so this throws a "Reference to non-existent field" error before relaxation can complete. Please gate these assignments behind nExt > 0 (or initialize empty extra-variable fields) so flux-only models do not crash.
Useful? React with 👍 / 👎.
| else | ||
| model.c=zeros(n,1); | ||
| % Stage 1: L2 min-norm via solveCobraLPCPLEX to get candidate blocked reactions | ||
| tol = 1e-10; |
There was a problem hiding this comment.
The L2 path now hard-codes tol = 1e-10, and that tolerance is used to form the FVA candidate set (abs(solution.full) < tol). If the QP/L2 solution has routine numerical noise above 1e-10 for truly blocked reactions, those reactions are excluded from candidateRxns and never checked by FVA, creating false negatives. This regresses robustness versus the previous solver-driven tolerance behavior; use solver feasibility/int tolerance (or a configurable tolerance) for candidate screening.
Useful? React with 👍 / 👎.
Summary:
Failed Tests:No failed tests ✨ Github Test Reporter by CTRF 💚 |
Codex P1 on #2651: the previous exclusion check asserted that no arc weight matched any excluded reaction's flux magnitude. In ecoli_core_model many non-excluded reactions share the biomass reaction's |v| (e.g. 0.87392), so the assertion is unstable across models and solvers and can produce false negatives. Replace it with a multiset comparison: replicate writePajekNet's filtering (biomass / objective name match, model.c != 0, |v| < fluxTol, demand/sink) to build the expected (src, dst, weight) arc list, parse the file's actual arcs, sort both, and assert equality. This validates exclusion by reaction identity and additionally catches missing arcs, swapped src/dst, and wrong weights. Expected weights are round-tripped through writePajekNet's '%g' format so the comparison is exact. Also closes a small gap in the previous test, which only matched 'biomass' in reaction names; now matches 'objective' as well, aligning with writePajekNet's own logic. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
fix(writePajekNet): directed Pajek output, tolerance filter, robust filtering & checks
Summary:
Failed Tests:No failed tests ✨ Github Test Reporter by CTRF 💚 |
8 participants
Please include a short description of enhancement here
I hereby confirm that I have:
developas a target branch (top left drop-down menu)(Note: You may replace [ ] with [X] to check the box)