Skip to content
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.

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

BGL - add CGAL::dijkstra_shortest_path(vs, vt, mesh) #8724

Open
wants to merge 24 commits into
base: master
Choose a base branch
from

Conversation

janetournois
Copy link
Member

@janetournois janetournois commented Jan 31, 2025

Summary of Changes

computes the shortest path between two vertices, using Dijkstra algorithm

Todo

  • write doc and integrate to
  • add np
  • add measure to define what "shortest" means
  • add to ref manual top page
  • use in demo code

Release Management

@janetournois janetournois added this to the 6.1-beta milestone Jan 31, 2025
@sloriot
Copy link
Member

sloriot commented Feb 6, 2025

Compilation error here

@MaelRL MaelRL added Not yet approved The feature or pull-request has not yet been approved. Small feature CHANGES.md not updated labels Feb 6, 2025
@afabri
Copy link
Member

afabri commented Feb 11, 2025

Compilation error here

What error ?

@sloriot
Copy link
Member

sloriot commented Feb 12, 2025

Successfully tested in CGAL-6.1-Ic-84

@github-actions github-actions bot removed the Tested label Mar 3, 2025

This comment was marked as outdated.

@janetournois janetournois marked this pull request as ready for review March 4, 2025 10:39
* \cgalParamDescription{a property map associating to each edge in the graph its weight or ``length''.
* The weights must all be non-negative.}
* \cgalParamType{a class model of `ReadablePropertyMap` with `boost::graph_traits<PolygonMesh>::%edge_descriptor`
* as key type and `FT` as value type.}
Copy link
Member

Choose a reason for hiding this comment

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

No need to introduce FT. It is probably specified in the BGL what has to be provided for "adding" and "comparing" weights.

Copy link
Member

Choose a reason for hiding this comment

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

I would like to suggest


*    \cgalParamType{a class model of `ReadablePropertyMap` with `boost::graph_traits<PolygonMesh>::%edge_descriptor`
*                   as key type and a value type which as specified in the names parameter `distance_map`of the function
                    <A href="https://www.boost.org/doc/libs/release/libs/graph/doc/dijkstra_shortest_paths.html">`boost::graph::dijkstra_shortest_paths()`</A>,
                    with any model of `RingNumberType` fulfilling the requirements. }

Copy link

github-actions bot commented Mar 4, 2025

The documentation is built. It will be available, after a few minutes, here: https://cgal.github.io/8724/v0/Manual/index.html

template<typename Graph,
typename OutputIterator,
typename NamedParameters = parameters::Default_named_parameters>
OutputIterator shortest_path_between_two_vertices(
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
OutputIterator shortest_path_between_two_vertices(
OutputIterator shortest_path(

Copy link
Member

Choose a reason for hiding this comment

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

@sloriot any opinion on the function name?

Copy link
Member

Choose a reason for hiding this comment

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

dijkstra_shortest_path() so that it's clear it's not geodesic

*
* \cgalNamedParamsBegin
* \cgalParamNBegin{edge_weight_map}
* \cgalParamDescription{a property map associating to each edge in the graph its weight or ``length''.
Copy link
Member

Choose a reason for hiding this comment

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

not sure how ``length'' will be interpreted by doxygen


/*!
* \ingroup PkgBGLTraversal
* Computes the shortest path between two vertices in a graph `g`.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* Computes the shortest path between two vertices in a graph `g`.
* computes the shortest path between two vertices in a graph `g`.

@janetournois janetournois changed the title BGL - add CGAL::shortest_path(vs, vt, mesh) BGL - add CGAL::dijkstra_shortest_paths(vs, vt, mesh) Mar 18, 2025
@janetournois janetournois changed the title BGL - add CGAL::dijkstra_shortest_paths(vs, vt, mesh) BGL - add CGAL::dijkstra_shortest_path(vs, vt, mesh) Mar 18, 2025
@janetournois
Copy link
Member Author

/build:v1

Copy link

The documentation is built. It will be available, after a few minutes, here: https://cgal.github.io/8724/v1/Manual/index.html

@afabri afabri added pre-approved For pre-approved small features. After 15 days the feature will be accepted. Ready to be tested and removed Not yet approved The feature or pull-request has not yet been approved. TODO labels Mar 20, 2025
@afabri
Copy link
Member

afabri commented Mar 20, 2025

/force-build:v1

Copy link

The documentation is built. It will be available, after a few minutes, here: https://cgal.github.io/8724/v1/Manual/index.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Pkg::BGL pre-approved For pre-approved small features. After 15 days the feature will be accepted. Ready to be tested Small feature Under Testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants