Skip to content

Fix dimension mismatch in Rosenbrock solver when constraints are present#929

Closed
Copilot wants to merge 3 commits intov2-review-dae-3-rosenbrock-integrationfrom
copilot/sub-pr-927
Closed

Fix dimension mismatch in Rosenbrock solver when constraints are present#929
Copilot wants to merge 3 commits intov2-review-dae-3-rosenbrock-integrationfrom
copilot/sub-pr-927

Conversation

Copy link
Contributor

Copilot AI commented Feb 11, 2026

When DAE constraints are enabled, variables_ has dimension state_size_ (species only) while temporary matrices (Ynew, K[], etc.) have dimension state_size_ + constraint_size_. Line 150 called Ynew.Copy(Y) which throws a runtime error on dimension mismatch.

Changes

  • Replace Ynew.Copy(Y) at line 150 with manual element-wise copy of species variables only
  • Use Ynew.NumRows() instead of Y.NumRows() in both copy loops for consistency with destination array
// Before: fails when constraint_size_ > 0
Ynew.Copy(Y);

// After: copy only species columns
Ynew.Fill(0.0);
for (std::size_t i_cell = 0; i_cell < Ynew.NumRows(); ++i_cell)
{
  for (std::size_t i_var = 0; i_var < state.state_size_; ++i_var)
  {
    Ynew[i_cell][i_var] = Y[i_cell][i_var];
  }
}

This mirrors the existing pattern at line 105-116 which already handled this correctly.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI and others added 2 commits February 11, 2026 07:38
Co-authored-by: boulderdaze <55209567+boulderdaze@users.noreply.github.com>
Co-authored-by: boulderdaze <55209567+boulderdaze@users.noreply.github.com>
Copilot AI changed the title [WIP] WIP Address feedback on V2 review dae 3 integration Fix dimension mismatch in Rosenbrock solver when constraints are present Feb 11, 2026
Copilot AI requested a review from boulderdaze February 11, 2026 07:43
@boulderdaze boulderdaze marked this pull request as ready for review February 11, 2026 09:53
@codecov-commenter
Copy link

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.69%. Comparing base (b92f583) to head (1e0a03f).
⚠️ Report is 5 commits behind head on v2-review-dae-3-rosenbrock-integration.

Additional details and impacted files
@@                            Coverage Diff                             @@
##           v2-review-dae-3-rosenbrock-integration     #929      +/-   ##
==========================================================================
+ Coverage                                   94.66%   94.69%   +0.03%     
==========================================================================
  Files                                          68       68              
  Lines                                        3485     3241     -244     
==========================================================================
- Hits                                         3299     3069     -230     
+ Misses                                        186      172      -14     

☔ View full report in Codecov by Sentry.
📢 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.

}

Ynew.Copy(Y);
// Copy only species variables from Y to Ynew.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the actual problem is that the temporary variables for the class are not properly sized. There is no reason Ynew and Y should ever have different sizes

@dwfncar
Copy link
Collaborator

dwfncar commented Feb 11, 2026

Closing, superceded by #931

@dwfncar dwfncar closed this Feb 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants