[jajhall]

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

[codecov]

Codecov Report

❌ Patch coverage is 95.82210% with 31 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.56%. Comparing base (b7e1ed9) to head (e862aa7).
⚠️ Report is 18 commits behind head on latest.

Files with missing lines	Patch %	Lines
highs/lp_data/HighsLp.cpp	90.44%	17 Missing ⚠️
highs/util/HighsMatrixUtils.cpp	93.20%	7 Missing ⚠️
highs/lp_data/Highs.cpp	91.30%	4 Missing ⚠️
check/TestLpValidation.cpp	99.27%	1 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.44%   75.56%   +0.11%     
==========================================
  Files         461      461              
  Lines      113172   113621     +449     
  Branches    17879    17903      +24     
==========================================
+ Hits        85385    85858     +473     
+ Misses      27499    27475      -24     
  Partials      288      288              

☔ 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.

[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]

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

[jajhall]

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

[jajhall]

Not much has changed since this was last reviewed by @fwesselm, other than all the changes due to Highs::logHeader() being in #3517

If a model with illegal data is passed, it is never retained (giving the user the chance to correct that illegal data). The incumbent model in the Highs instance is cleared.

Many changes to unit tests are because the "group" name in several files was "[highs_data]" - dating way back to before I knew that this was valuable - and have been given names consistent with the file name.