Skip to content

Fix inversion crossover bug with pdp and prize collection #179

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
rapids-bot merged 7 commits into from
Jul 3, 2025
Merged

Fix inversion crossover bug with pdp and prize collection #179

rapids-bot merged 7 commits into from
Jul 3, 2025

Conversation

@hlinsen
Copy link
Contributor

@hlinsen hlinsen commented Jul 2, 2025 •
edited
Loading

IX is assuming that all nodes are present in solution b. We can fill the missing node in b to ensure sorting will keep the precedence.
This PR also adds a check for initial solutions with PDP
Closes: #177

@hlinsen hlinsen requested a review from a team as a code owner July 2, 2025 14:59
@hlinsen hlinsen requested review from akifcorduk and rg20 July 2, 2025 14:59
@hlinsen hlinsen added bug Something isn't working non-breaking Introduces a non-breaking change labels Jul 2, 2025
rg20
rg20 reviewed Jul 2, 2025
View reviewed changes
@akifcorduk
Copy link
Contributor

akifcorduk commented Jul 2, 2025 •
edited
Loading

Are we using a different clang style file now? It is hard to find the file where the actual code change happens. If there is no mistake on the styling, i think we should reformat whole repo with a PR.

Also could you please open an issue to make sure we implement the IX for PDP? Do we know the quality impact of disabling IX? If it is a significant decrease in quality, we should implement the fix instead of disabling it.

@hlinsen
Copy link
Contributor Author

hlinsen commented Jul 2, 2025

Are we using a different clang style file now? It is hard to find the file where the actual code change happens. If there is no mistake on the styling, i think we should reformat whole repo with a PR.

Also could you please open an issue to make sure we implement the IX for PDP? Do we know the quality impact of disabling IX? If it is a significant decrease in quality, we should implement the fix instead of disabling it.

I think it was never meant to be enabled. I can check on homberger.

@anandhkb anandhkb added this to the 25.08 milestone Jul 2, 2025
@hlinsen hlinsen changed the title Disable IX for PDP Fix inversion crossover bug with pdp and prize collection Jul 3, 2025
rg20
rg20 approved these changes Jul 3, 2025
View reviewed changes
Copy link
Contributor

@rg20 rg20 left a comment

Choose a reason for hiding this comment

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

LGTM!

akifcorduk
akifcorduk reviewed Jul 3, 2025
View reviewed changes
@hlinsen
Copy link
Contributor Author

hlinsen commented Jul 3, 2025

/merge

@rapids-bot rapids-bot bot merged commit d204b1d into NVIDIA:branch-25.08 Jul 3, 2025
141 of 142 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@akifcorduk akifcorduk akifcorduk left review comments

@rg20 rg20 rg20 approved these changes

Assignees

No one assigned

Labels

bug Something isn't working non-breaking Introduces a non-breaking change

Projects

None yet

Milestone

25.08

Development

Successfully merging this pull request may close these issues.

[BUG] cudaErrorIllegalAddress

4 participants

@hlinsen @akifcorduk @rg20 @anandhkb