Fix Negative Curvature Status Overriding in CR.jl

[farhadrclass]

Description:
This PR addresses an issue in CR.jl where the "nonpositive curvature" status was never set as intended. The problem arose from the current evaluation order:

solved = resid_decrease || npcurv || on_boundary
...
npcurv  && (status = "nonpositive curvature")
solved  && (status = "solution good enough given atol and rtol")

Because solved is defined as resid_decrease || npcurv || on_boundary, even when npcurv is true, solved also becomes true, leading to the "solution good enough given atol and rtol" status being assigned afterward. This inadvertently overrides the negative curvature status, meaning the "nonpositive curvature" status would never actually be set.

Changes Made:

• Reordered and Refactored Conditions:
  Modified the logic so that the negative curvature condition (npcurv) is checked and assigned before the general solved condition. This ensures that if negative curvature is detected, the "nonpositive curvature" status takes precedence.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.66%. Comparing base (9536ef7) to head (71aa227).
Report is 51 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #970      +/-   ##
==========================================
- Coverage   94.68%   93.66%   -1.02%     
==========================================
  Files          45       47       +2     
  Lines        8027     9205    +1178     
==========================================
+ Hits         7600     8622    +1022     
- Misses        427      583     +156     

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

[farhadrclass]

One side note I noticed, is that we never test CR with radius > 0, I think we need a bug for that?

[dpo]

Right, but in trust-region methods, we would want “on trust-region boundary” to be the final status.

[farhadrclass]

@dpo so the issue here is that we want “on trust-region boundary” be the status?
In CR the flag is only set once

 elseif pAp > 0 && ρ > 0  # no negative curvature
          (verbose > 0) && @printf(iostream, "positive curvatures along p and r. pᴴAp = %8.1e and rᴴAr = %8.1e\n", pAp, ρ)
          α = ρ / kdotr(n, q, Mq)
          if α ≥ t1
            α = t1
            on_boundary = true
          end

which is not the case in this test, since A is a diagonal with very -ve values

[farhadrclass]

@dpo ?

[farhadrclass]

@dpo
When running the following test:

# Test linesearch
A, b = symmetric_indefinite(FC=FC)
x, stats = cr(A, b, linesearch=true)
@test stats.status == "nonpositive curvature"
@test real(dot(x, A * x)) ≤ 0   # <-- this test fails

# Test on trust-region boundary when radius > 0
A, b = symmetric_indefinite(FC=FC, shift=5)
x, stats = cr(A, b, radius=one(Float64))
@test stats.status == "on trust-region boundary"
@test real(dot(x, A * x)) ≤ 0
@test norm(x) ≈ 1.0

The first test is failing while the second one passes. I suspect this is because when linesearch is enabled, the solver returns the final x instead of the one that produced the negative curvature.

Once the PR for MINRES is accepted, I plan to return npc_dir for the CR solver and update the tests accordingly.