Skip to content

Fix Rosenbrock MINUS_ONE bug - #535

Merged
nkoukpaizan merged 2 commits into
developfrom
alex/fix-rosenbrock-negative
Aug 14, 2026
Merged

Fix Rosenbrock MINUS_ONE bug#535
nkoukpaizan merged 2 commits into
developfrom
alex/fix-rosenbrock-negative

Conversation

@alexander-novo

Copy link
Copy Markdown
Collaborator

Description

Rosenbrock was using wrong value of alpha when evaluating model Jacobian - there was an extra minus sign. This wasn't caught by the test because the test was also incorrectly using alpha in its own Jacobian - there was a missing minus sign. These cancelled out and cause correct behavior when Rosenbrock was applied to the test problem.

Some documentation has been added to assist with understanding the issue.

Proposed changes

The MINUS_ONE in Rosenbrock's model Jacobian evaluation has been changed to a ONE, and the test has been updated to compute its Jacobian in a way that agrees with the rest of GridKit.

Checklist

Put an x in the boxes that apply. You can also fill these out after creating
the PR. If you're unsure about any of them, don't hesitate to ask. We're here
to help! This is simply a reminder of what we are going to look for before
merging your code.

  • All tests pass.
  • Code compiles cleanly with flags -Wall -Wpedantic -Wconversion -Wextra.
  • The new code follows GridKit™ style guidelines.
  • There are unit tests for the new code.
  • The new code is documented.
  • The feature branch is rebased with respect to the target branch.
  • I have updated CHANGELOG.md to reflect the changes in this PR. If this is a minor PR that is part of a larger fix already included in the file, state so.

This is a fix to the rosenbrock integrator, which is already included in the changelog

Further comments

@alexander-novo
alexander-novo requested a review from pelesh August 11, 2026 22:00
@alexander-novo alexander-novo self-assigned this Aug 11, 2026
@alexander-novo alexander-novo added bug Something isn't working documentation Improvements or additions to documentation labels Aug 11, 2026
Rosenbrock was using wrong value of `alpha` when evaluating model Jacobian - there was an extra minus sign. This wasn't caught by the test because the test was also incorrectly using alpha in its own Jacobian - there was a missing minus sign. These cancelled out and cause correct behavior when Rosenbrock was applied to the test problem.
@alexander-novo
alexander-novo force-pushed the alex/fix-rosenbrock-negative branch from 2475cca to fc3075c Compare August 13, 2026 15:20
@nkoukpaizan
nkoukpaizan merged commit 9f84a03 into develop Aug 14, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants