Skip to content

Update kkt_errors!#21

Open
klamike wants to merge 5 commits intomainfrom
mk/dualobj
Open

Update kkt_errors!#21
klamike wants to merge 5 commits intomainfrom
mk/dualobj

Conversation

@klamike
Copy link
Copy Markdown
Collaborator

@klamike klamike commented Feb 13, 2026

Before this, the code was actually computing -1 * dual objective, it is why the gap was primal + dual instead of primal - dual. This PR updates the code to compute the dual objective with proper sign, which also cleans up the kernels (no more negation inside the positive_part/negative_part)

@klamike klamike requested a review from gdalle February 13, 2026 20:47
@klamike klamike changed the title Update dual objective in KKTErrors Update kkt_errors! Feb 13, 2026
@gdalle
Copy link
Copy Markdown
Member

gdalle commented Feb 18, 2026

The reason for my choice of notations was to stick to the published papers as much as possible. Is there a stronger reason to deviate?

@klamike
Copy link
Copy Markdown
Collaborator Author

klamike commented Feb 18, 2026

Which paper? https://arxiv.org/pdf/2106.04756 equation 1, https://arxiv.org/pdf/2311.12180 equation 5 uses the form I am suggesting. I do believe that this form is more popular in the literature more broadly, definitely it is the one that I prefer.

@gdalle
Copy link
Copy Markdown
Member

gdalle commented Feb 23, 2026

I think I used the notations from later papers such as

The team around Haihao Lu changed their notations sometime between the first cuPDLP.jl paper and these

@klamike
Copy link
Copy Markdown
Collaborator Author

klamike commented Feb 23, 2026

I'm not sure I follow. How is it more general? It is just switching/optimizing the computation of $|x|^+$ and $|x|^-$, and computing dobj rather than -dobj. It does look like cuPDLPx is computing dobj since they have primal - dual (coolpdlp had primal + dual before this PR):
https://github.com/MIT-Lu-Lab/cuPDLPx/blob/c28d2c9032f42c7cd0d463f9da4f4c480bbad2e3/src/utils.cu#L883

@klamike
Copy link
Copy Markdown
Collaborator Author

klamike commented Feb 23, 2026

To me, min(x, 0) and max(x, 0) is much clearer than -max(-x, 0) and -min(-x, 0). Furthermore, the current computation is relying on the fact that we can delay the outer negations all the way until we finally compute the gap. It just seems overly convoluted to me, and I don't understand why it helps to write it this way.

@gdalle gdalle mentioned this pull request Mar 9, 2026
@klamike
Copy link
Copy Markdown
Collaborator Author

klamike commented Mar 31, 2026

@gdalle friendliest of pings :) have you had a chance to take a look at this?

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.

2 participants