Improve Feng, PNC algorithm

[kappybar]

• Fix several bugs of PNC algorithm.
• Improve implementation of Feng algorithm.
• Add PNC as a supported algorithm in shortest_simple_paths

📝 Checklist

• The title is concise and informative.
• The description explains in detail what this PR is about.
• I have linked a relevant issue or discussion.
• I have created tests covering the changes.
• I have updated the documentation and checked the documentation preview.

⌛ Dependencies

#40364

[dcoudert]

Some failing doctests https://github.com/sagemath/sage/actions/runs/16621950853/job/47028268037?pr=40510#step:11:348

[dcoudert]

How difficult would it be to avoid the duplication of methods shortest_path_to_green and ancestor_idx_func ?

[kappybar]

I think the easiest option is to integrates two functions and use common code except for while candidate_paths.size(): section. The current code is almost same, and can be exactly same.

The plan is to integrate feng_k_shortest_simple_paths, pnc_k_shortest_simple_paths into one function such as nc_k_shortest_simple_paths and add argument algorithm=(normal|postponed) to distinguish two algorithms.

[dcoudert]

I suggest to start with the "easy" parts that can be shared. I assume that mixing Feng and PNC is much more involved and prone to errors.

[kappybar]

Now, L1103-L1236 is exactly same as L1401-L1534 in path_enumeartion.pyx.

def nc_k_shortest_simple_paths(self,...., algorithm="normal"):
     ...
     common codes for feng and pnc
     ...
     if algorithm == "normal":
           THE FENG ALGORITHM (L1237-L1287 in path_enumeration.pyx)
     elif algorithm == "postponed":
           THE PNC ALGORITHM  (L1535-L1597 in path_enumeration.pyx)

will keep two algorithms independent, so will not make serious error.

[kappybar]

I am not sure how to handle deleted functions (e.g., Feng_k_shortest_simple_paths, pnc_k_shortest_simple_paths). Should I raise deprecated error in calling these functions?

[dcoudert]

An option is to keep both methods feng_k_shortest... and pnc_k_shortest... with only few tests and doing only yield from nc_k_shortest....
This way we avoid the need for deprecation.
Furthermore, it ease the manipulation of the different algorithms if one wants to do some benchmark.

[dcoudert]

1. this part can be simplified as follows

    if algorithm in ("Feng", '"PNC"):
        if not self.is_directed():
            raise ValueError(f"{algorithm}'s algorithm works only for directed graphs")

        yield from nc_k_shortest_simple_paths(self, source=source, target=target,
                                              weight_function=weight_function,
                                              by_weight=by_weight, check_weight=check_weight,
                                              report_edges=report_edges,
                                              labels=labels, report_weight=report_weight,
                                              postponed=algorithm == "PNC")

    elif algorithm == "Yen":
        yield from yen_k_shortest_simple_paths(self, source=source, target=target,
                                               weight_function=weight_function,
                                               by_weight=by_weight, check_weight=check_weight,
                                               report_edges=report_edges,
                                               labels=labels, report_weight=report_weight)

2. instead of using parameter algorithm in nc_k_shortest_simple_paths, I propose to simply use a boolean postponed=True|False

[dcoudert]

add an empty line before/after to improve readability.

[dcoudert]

add empty line before/after

[dcoudert]

you should update the branch on the last beta now that #40364 has been merged

[github-actions]

Documentation preview for this PR (built with commit dfa5625; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

[dcoudert]

LGTM.

Further improvements are certainly possible, but it is enough for this PR.