`Highs::passModel()` and `Highs::passHessian()` now return `HighsStatus::kWarning` if any of the `HighsLp` vectors or `HighsHessian::start_` have greater size than required by `HighsLp::num_col_` and `HighsLp::num_row_` by jajhall · Pull Request #3372 · ERGO-Code/HiGHS

jajhall · 2026-10-07T19:18:35Z

Description

Observing an assert when experimenting with #3364, I decided that trimming HighsLp vectors that are too large (and logging) would be good for users and developers.

lpDimensionsOk and its parameter list have been modified to be consistent with assessHessianDimensions and "legal" used to distinguish them from "trim", as in trimHessianDimension

NB @fwesselm I spotted that if num_col = iCol then lp_.col_names_[num_col] = std::move(lp_.col_names_[iCol]); leads to lp_.col_names_[num_col] being an empty string.

Checklist

I have read the contributing guidelines
This PR targets the latest branch
Tests are passing
Documentation was updated where relevant
This PR is not primarily AI-generated (per the AI contributions policy in CONTRIBUTING.md)

Related issue

Closes #3366

…mensions and legalHessianDimensions

…Warning if any of the HighsLp vectors or HighsHessian::start_ have greater size than required by HighsLp::num_col_ and HighsLp::num_row_. They are resized, with logging when this occurs

codecov · 2026-10-07T19:33:29Z

Codecov Report

❌ Patch coverage is 96.58738% with 33 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.19%. Comparing base (6af93b8) to head (b98344c).
⚠️ Report is 21 commits behind head on latest.

Files with missing lines	Patch %	Lines
highs/lp_data/HighsLp.cpp	90.39%	17 Missing ⚠️
highs/lp_data/Highs.cpp	96.40%	6 Missing ⚠️
highs/util/HighsMatrixUtils.cpp	93.75%	6 Missing ⚠️
check/TestLpValidation.cpp	99.06%	2 Missing ⚠️
highs/model/HighsHessian.h	50.00%	1 Missing ⚠️
highs/model/HighsModel.cpp	93.75%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           latest    #3372      +/-   ##
==========================================
+ Coverage   75.02%   75.19%   +0.17%     
==========================================
  Files         460      460              
  Lines      111862   112441     +579     
  Branches    17746    17772      +26     
==========================================
+ Hits        83921    84548     +627     
+ Misses      27659    27611      -48     
  Partials      282      282

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

fwesselm

Thanks, @jajhall. Just a couple of comments from me.

…; trimDimensions in HighsLp and HighsHessian

… legalDimensions

…rimDimensions

…atrixDimension and trimHessianDimension

…from calling parameters; created HighsSparseMatrix::trimDimensions

…ests

…3366

…ing it to be set when each HiGHS "entry" method is called. Hence Highs::logHeader() generalised to Highs::onFirstEntry(), and Highs::written_log_header_ generalised to Highs::made_first_call_

…x-3366

…ned matrix, and added some asserts to check the partitioned matrix in primal/dual simplex

jajhall · 2026-10-12T16:27:42Z

Spotted quite a few methods in the Highs class that, if called first after creating the Highs instance would yield logging before highsLogHeader is called. These methods are not logical to call before the typical "first method", but the changes keep the code consistent. This means that many more methods in the Highs class are not const - since calling highsLogHeader changes the data member used to record whether it's been called.

highsLogHeader is now called from the more generic Highs::onFirstCall that can be used to do other stuff in due course

…x-3366

…3366

fwesselm · 2026-10-13T08:17:23Z

Just a thought, could mutable be used to keep these methods const?

mutable bool made_first_call_ = false;
mutable bool written_log_header_ = false;

The only question would be if any of the const methods could be called concurrently by different threads.

jajhall · 2026-10-13T09:48:24Z

Just a thought, could mutable be used to keep these methods const?
mutable bool made_first_call_ = false;
mutable bool written_log_header_ = false;
The only question would be if any of the const methods could be called concurrently by different threads.

If it's just a question of writing the log header - as it is currently - then I think I'm correct in saying that the worst that could happen is that it gets written twice. This is not a problem, as anyone with multiple threads using the same HiGHS instance will either get unreadable logging, or will be sending logging to different files for each thread. In the latter case, having the header at the top of the file (or not) isn't a bad thing, even if they are parsing the log file.

Am I right in thinking that the advantage of marking a method as const is that it can be called concurrently by different threads (because data members shared between threads can't be changed)? If so, then I can see the value in just using onFirstCall to write the logging header and making written_log_header_ mutable. This will mean that many methods can become const again. The performance gain won't be much as, obviously, the major computation occurs in methods that can't be const.

…3366

fwesselm

@jajhall, just one more question. Would it be possible to have a

mutable std::once_flag log_header_flag_;

member in Highs.h:

void Highs::logHeader() const {
    // Defer logging the header until output is switched on
    if (!*options_.log_options.output_flag) return;
    std::call_once(log_header_flag_, [this]() {
      highsLogHeader(options_.log_options, options_.log_githash);
    });
  }

This would allow for making the methods const again and every Highs instance would have its own flag.

fwesselm · 2026-10-14T06:41:29Z

+void Highs::logHeader(const bool log_header) {
+  if (!options_.output_flag) return;
+  //  if (!*options_.log_options.output_flag) return;
+  static std::once_flag flag;


Sorry, I am not an expert in this, but the static std::once_flag is shared by all Highs instances, so the header is now only logged once per process: if a process creates two Highs instances, only the first one logs the header.

Thanks, I'm happy with this. It means that a lot of methods are const again, and we don't have to argue that the mutable value doesn't really compromise thread safety.

fwesselm · 2026-10-14T06:47:15Z

@@ -3181,6 +3181,8 @@ HighsStatus Highs::computeIllConditioning(
  Highs conditioning;
  const bool dev_conditioning = false;
  conditioning.setOptionValue("output_flag", false);  // dev_conditioning);


If output_flag is false, logHeader(false) does immediately return, correct? Is the parameter log_header used only here?

if (!options_.output_flag) return;

fwesselm · 2026-10-14T06:51:13Z

+  return status;
+}
+
+HighsStatus HighsLp::assess(const HighsOptions& options) {


Since matrix dimensions may not have been set, it may be good to call this->setMatrixDimensions() here?

fwesselm · 2026-10-14T06:53:27Z

 }

+HighsStatus HighsSparseMatrix::assess(const HighsOptions& options) {
+  const bool sum_duplicates = false;


Maybe return a warning/error if there are no vectors but starting points are set? Maybe there is another case that could be handled with a warning...

if (this->numVec() == 0 && this->start_.size() > 1) ...

fwesselm · 2026-10-14T06:55:53Z

-    lp.a_matrix_.value_.resize(lp_num_nz);
+
+  // Matrix should have been trimmed in lp.a_matrix_.assess
+  assert(lp.a_matrix_.trimDimensions(log_options) == HighsStatus::kOk);


This would only trim if assertions are enabled (debug mode)?

…fix-3366

fwesselm · 2026-10-14T07:17:13Z

@jajhall, I merged with latest (there were conflicts)

…x-3366

…for presolve-initial-sweep-all

…cols with illegal/small values returns the correct status and (if illegal) isn't added

jajhall · 2026-10-14T22:25:46Z

Sorry @fwesselm, this is getting horrible to review. Don't look at this yet, as I'll write some commentary about the reasoning for the changes

…rmat.cpp

Julian Hall added 5 commits October 7, 2026 12:04

Updated assessLpDimensions to legalLpDimensions

1fb31c4

Renamed lpDimensionsLegal to legalLpDimensions matching legalMatrixDi…

ff8f348

…mensions and legalHessianDimensions

Finally got the example to trigger the two asserts before fixing

992c226

Now trimming HighsLp data in assessLp

86710bd

Highs::passModel() and Highs::passHessian() now return HighsStatus::k…

79f0178

…Warning if any of the HighsLp vectors or HighsHessian::start_ have greater size than required by HighsLp::num_col_ and HighsLp::num_row_. They are resized, with logging when this occurs

jajhall requested a review from fwesselm October 7, 2026 19:18

jajhall self-assigned this Oct 7, 2026

jajhall added the Code-quality label Oct 7, 2026

jajhall mentioned this pull request Oct 7, 2026

What happens if there is extra HighsLp data? #3366

Closed

fwesselm reviewed Oct 8, 2026

View reviewed changes

Julian Hall and others added 11 commits October 8, 2026 09:08

Set up legalDimensions in HighsSparseMatrix, HighsLp and HighsHessian…

fa57bef

…; trimDimensions in HighsLp and HighsHessian

Replaced legalLpDimensions and legalHessianDimensions by class method…

68b62a4

… legalDimensions

Replaced trimLpDimensions and trimHessianDimensions by class method t…

68885fe

…rimDimensions

Formatted, but LP-illegal-empty-start-ok fails

17b726f

Now write trimDimension for HighsSparseMatrix and HighsHessian, trimM…

786d4f4

…atrixDimension and trimHessianDimension

Introduced name_ for HighsSparsematrix and HighsHessian, removing it …

39edc69

…from calling parameters; created HighsSparseMatrix::trimDimensions

Merge branch 'latest' into fix-3366

449ade2

LP-illegal-start passes, but what else fails?

22f4569

Debugged trim methods, and exposed incorrect start vector in 5 unit t…

4071c97

…ests

Merged latest into this branch and resolved minor conflict

97f7048

Merge branch 'latest' of https://github.com/ERGO-Code/HiGHS into fix-…

fc29f2b

…3366

fwesselm reviewed Oct 12, 2026

View reviewed changes

jajhall and others added 7 commits October 12, 2026 10:16

Addressed the simple changes

0c4eed3

Added pointer to log_options to HighsLp and HighsSparseMatrix, requir…

7978f26

…ing it to be set when each HiGHS "entry" method is called. Hence Highs::logHeader() generalised to Highs::onFirstEntry(), and Highs::written_log_header_ generalised to Highs::made_first_call_

Transfer to fletcher

0e1b26b

Merge branch 'fix-3366' of https://github.com/ERGO-Code/HiGHS into fi…

cf92328

…x-3366

HighsLp and HighsSparseMatrix don't need HighsLogOptions

d7d461a

Cleaned out log_options pointer stuff

b15c84e

All unit tests pass and are silent

51fd0c0

Julian Hall added 2 commits October 12, 2026 17:06

Fixed potential read error if assessMatrix were called for a partitio…

0c81878

…ned matrix, and added some asserts to check the partitioned matrix in primal/dual simplex

Still need to distinguish made_first_call_ and written_log_header_

41f0429

fwesselm added 2 commits October 13, 2026 09:58

Merge branch 'fix-3366' of https://github.com/ERGO-Code/HiGHS into fi…

37d6671

…x-3366

Merge branch 'latest' of https://github.com/ERGO-Code/HiGHS into fix-…

9d62be7

…3366

Julian Hall and others added 8 commits October 13, 2026 13:57

Added std::call_once wrapper for highsLogHeader call

3fcdea8

Renamed oFirstCall back to logHeader

5d7ab2f

Formatted

199cba6

Restored const to Highs methods that were previously const

b6ca673

Merged latest into this branch and resolved minor conflict

b8d2cab

Made Highs methods const where possible

37ddfbd

Fixing LP-extreme-coefficient

66bf638

Merge branch 'latest' of https://github.com/ERGO-Code/HiGHS into fix-…

da820be

…3366

fwesselm reviewed Oct 14, 2026

View reviewed changes

Merge branch 'fix-3366-jh' of https://github.com/fwesselm/HiGHS into …

79df378

…fix-3366

Julian Hall added 7 commits October 14, 2026 09:39

Merge branch 'fix-3366' of https://github.com/ERGO-Code/HiGHS into fi…

8fb3dbd

…x-3366

Should models with a lower (upper) bound of (inf) (-inf) be accepted?

7b36a23

Removed the old development presolve-initial-sweep and used its name …

7a86ab5

…for presolve-initial-sweep-all

Merge branch 'latest' into fix-3366

9f5907a

Added test add-extreme-values to ensure that (attempting to) add row/…

1002c78

…cols with illegal/small values returns the correct status and (if illegal) isn't added

Do we need cleanBounds and processedBoundsFindsInfeasibility?

6689384

Modified unit test resturn status when infeasible bounds are passed

d426f0d

Merged latest into this branch and resolved conflicts in TestLpFileFo…

b98344c

…rmat.cpp

Conversation

jajhall commented Oct 7, 2026 • edited by fwesselm Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Related issue

Uh oh!

codecov Bot commented Oct 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

fwesselm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jajhall commented Oct 12, 2026

Uh oh!

fwesselm commented Oct 13, 2026

Uh oh!

jajhall commented Oct 13, 2026

Uh oh!

fwesselm left a comment

Choose a reason for hiding this comment

Uh oh!

fwesselm Oct 14, 2026

Choose a reason for hiding this comment

Uh oh!

jajhall Oct 14, 2026

Choose a reason for hiding this comment

Uh oh!

fwesselm Oct 14, 2026

Choose a reason for hiding this comment

Uh oh!

fwesselm Oct 14, 2026

Choose a reason for hiding this comment

Uh oh!

fwesselm Oct 14, 2026

Choose a reason for hiding this comment

Uh oh!

fwesselm Oct 14, 2026

Choose a reason for hiding this comment

Uh oh!

fwesselm commented Oct 14, 2026

Uh oh!

jajhall commented Oct 14, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jajhall commented Oct 7, 2026 •

edited by fwesselm

Loading

codecov Bot commented Oct 7, 2026 •

edited

Loading