Feasibility jump: fix #3302 and #3303

[BenChampion]

Description

1. Handles jump candidates with negative constraint coefficients Feasibility Jump: rows with a negative coefficient are skipped when computing the jump value #3302
2. Corrects sign of objective contribution Feasibility Jump: the objective term of the move score has the wrong sign #3303
3. Removes double-counting for equality constraints in jump candidate handling (noticed when fixing the other two)

Also amends comments.

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 issues

Closes #3302
Closes #3303

[BenChampion]

@jajhall I'd like to performance test this - we can chat about it tomorrow

[jajhall]

Good spot. Were these errors in the original code?

[fwesselm]

Looks good, @BenChampion!

[BenChampion]

Good spot.

Credit to @ spoorendonk!

Were these errors in the original code?

Yes. I did check for interactions with new functionality, though.

[Opt-Mucca]

I guess this would mostly change the quality of solutions FJ was returning (not the general ratio of success)?

[BenChampion]

I guess this would mostly change the quality of solutions FJ was returning (not the general ratio of success)?

It's conceivable that we might see a bit of both, because the bug meant it was basically missing constraints when trying to reduce infeasibilities.

The FJ paper didn't test objective improvement of the reference implementation, but it's not clear to me whether that meant using a zero objective or something else, which would influence the size of the impact we'll see on solution quality.

We'll find out!

[BenChampion]

This is good to merge!

Benchmarking (with a focus on early feasible solutions) shows decent improvements on both time and solution quality.

Thanks to @Opt-Mucca for the scripts that made this very easy!

Details (for those interested)

With the fork-point fd860f9b as baseline, I ran the MIPLIB2017 benchmark 240 (3 random seeds) with

time_limit=120
mip_rel_gap=1

Note that these are quite different settings from typical performance runs, on which the overall impact may be much smaller.

Subset	# Instance-Seed Pairs
All	720
Gap < 1 (test and baseline)	514
Gap < 1 (test and baseline) AND test/baseline objectives differ	146

On the subset of 514, the time ratio (shifted geometric means, shift 1) was 0.94 in favour of the change
(Node ratio has little meaning here.) Nearly 2/3 of the objectives didn't change, but among the subset of 146 above that did, the gap difference (shifted geometric means, shift 1) was 0.15 in favour of the change.