Skip to content

Speed up ensureColwise and ensureRowwise - #3485

Open
filikat wants to merge 4 commits into
latestfrom
transpose-2
Open

filikat wants to merge 4 commits into
latestfrom
transpose-2

Conversation

@filikat

@filikat filikat commented Oct 28, 2026

Copy link
Copy Markdown
Collaborator

Description

Currently, ensureRowwise and ensureColwise make an internal copy of the matrix and then build the correct format in the original storage of the matrix. We can avoid the copy by building the correct format in a temporary object and move assigning to the original matrix. *this = oppositeFormat() does this and does not need an explicit std::move, since the result of oppositeFormat() is a temporary. However, it does need the move assignment operation to be declared, otherwise it would fall back to a deep copy. This is why the special operations of HighsSparseMatrix are explicitly defaulted, so that if someone defines a custom copy operation, the move operations do not silently vanish.

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)

@filikat filikat self-assigned this Oct 28, 2026
@codecov

codecov Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.22%. Comparing base (1001c16) to head (310850c).
⚠️ Report is 91 commits behind head on latest.

Additional details and impacted files
@@            Coverage Diff             @@
##           latest    #3485      +/-   ##
==========================================
- Coverage   75.24%   75.22%   -0.03%     
==========================================
  Files         460      460              
  Lines      112567   112523      -44     
  Branches    17836    17827       -9     
==========================================
- Hits        84697    84641      -56     
- Misses      27588    27600      +12     
  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.

@filikat
filikat marked this pull request as ready for review October 28, 2026 17:32
@filikat
filikat requested a review from jajhall October 28, 2026 17:32
assert(static_cast<HighsInt>(start_.size()) >= num_col_ + 1);
assert(static_cast<HighsInt>(index_.size()) >= numNz());
assert(static_cast<HighsInt>(value_.size()) >= numNz());
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can these be == rather than >=, as not keeping sizes tight has caused segfault in presolve (for example)

assert(numNz() == num_nz);
assert(static_cast<HighsInt>(start_.size()) >= num_row_ + 1);
assert(static_cast<HighsInt>(index_.size()) >= numNz());
assert(static_cast<HighsInt>(value_.size()) >= numNz());

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can these be == rather than >=, as not keeping sizes tight has caused segfault in presolve (for example)

This branch has not been deployed

No deployments
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.

2 participants