Fix initial solution accepted index update in generate_initial()

[SomovMike]

Description

Fix index handling when selecting initial solutions in generate_initial().

• Adjust while-loop bound from injection_it < n_sol to injection_it < n_sol - 1 to prevent advancing past the last valid index.
• Change candidate selection to use pre-increment (++injection_it) when assigning to temp_pair.first, ensuring the selected solution and next_injection remain aligned.

After the loop, next_injection now points to the same candidate stored in temp_pair.first, preventing incorrect updates to accepted.

Issue

Closes #338

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[rgsl888prabhu]

/ok to test 3e70c70

[rgsl888prabhu]

Thank you @SomovMike for the PR, one of us will review your PR shortly.

[hlinsen]

Thanks for reporting the bug and submitting the PR!
Could you please add the failing test here https://github.com/NVIDIA/cuopt/blob/branch-25.10/python/cuopt/cuopt/tests/routing/test_initial_solutions.py? Assuming you have the code ready.

Regarding the fix, after the change in your example we would be skipping sol[0] and add try to add it while omitting the target vehicle check.
We should set accepted in the for loop:

            while (injection_it < injection_info.n_sol &&
                   temp_pair.first.sol.get_n_routes() > target_vehicles_) {
              injection_info.accepted[injection_it] = 0;
              temp_pair.first = injection_info.solutions[injection_it++];
            }

And here check that next_injection < injection_info.n_sol as we could skip all solutions due to vehicle count and try to insert temp.first then write to accepted thus the out of bounds.

[SomovMike]

Hi @hlinsen, thank you for your patience, and apologies for the delayed response.

Unfortunately, I don’t have a minimal failing test case available, as I encountered the issue on a large instance while working on my own implementation. I hope that’s not a problem, since the issue seems fairly clear from the code itself.

Regarding the fix, I think it depends on how you envision the logic of the algorithm. If the primary objective is to minimize the number of vehicles, and a solution with fewer vehicles has already been found, then it might make sense to skip all initial solutions with more vehicles.

That said, in my opinion, it could still be beneficial to include those initial solutions with more vehicles, since the crossover algorithm might be able to leverage them to generate improved solutions.

[hlinsen]

In cuOpt we always assume minimization unless vehicle_fixed_costs are set or we use a fixed vehicle count in this case it does not change throughout the solve. Before the vehicle skip loop we are explicitly checking if vehicle fixed costs are set so following that logic we would not want to insert these vehicles at all.

[github-actions]

🔔 Hi @anandhkb @SomovMike, this pull request has had no activity for 7 days. Please update or let us know if it can be closed. Thank you!

If this is an "epic" issue, then please add the "epic" label to this issue.
If it is a PR and not ready for review, then please convert this to draft.
If you just want to switch off this notification, then use the "skip inactivity reminder" label.

[github-actions]

🔔 Hi @anandhkb @SomovMike, this pull request has had no activity for 7 days. Please update or let us know if it can be closed. Thank you!

If this is an "epic" issue, then please add the "epic" label to this issue.
If it is a PR and not ready for review, then please convert this to draft.
If you just want to switch off this notification, then use the "skip inactivity reminder" label.

[rgsl888prabhu]

@SomovMike May I request you to address review comment from @hlinsen