add cutoff versions of shortest path functions #2362

GroteGnoom · 2023-06-10T08:21:58Z

codecov · 2023-06-10T08:26:40Z

Codecov Report

Merging #2362 (d7d6b7f) into master (e38117b) will increase coverage by 0.03%.
The diff coverage is 97.56%.

@@            Coverage Diff             @@
##           master    #2362      +/-   ##
==========================================
+ Coverage   83.50%   83.54%   +0.03%     
==========================================
  Files         377      377              
  Lines       61909    61932      +23     
==========================================
+ Hits        51700    51742      +42     
+ Misses      10209    10190      -19

Impacted Files	Coverage Δ
src/paths/all_shortest_paths.c	`96.46% <80.00%> (-0.79%)`	⬇️
src/paths/dijkstra.c	`96.61% <100.00%> (+0.27%)`	⬆️
src/paths/unweighted.c	`95.07% <100.00%> (+0.30%)`	⬆️

... and 4 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e38117b...d7d6b7f. Read the comment docs.

GroteGnoom · 2023-07-16T08:48:25Z

I think this https://dev.azure.com/igraph-team/igraph/_build/results?buildId=5553&view=logs&jobId=bbfa2fa6-a202-51ab-3559-6013e9e5f686&j=bbfa2fa6-a202-51ab-3559-6013e9e5f686&t=46cc7337-1682-57f5-be99-ed9c70724455

[191/194] Resolving includes in DocBook XML source
FAILED: doc/igraph-docs-with-resolved-includes.xml /home/vsts/work/1/s/build/doc/igraph-docs-with-resolved-includes.xml 
cd /home/vsts/work/1/s/build/doc && /usr/bin/xmllint --xinclude --output igraph-docs-with-resolved-includes-tmp.xml igraph-docs.xml && /usr/bin/python3.10 /home/vsts/work/1/s/tools/removeexamples.py igraph-docs-with-resolved-includes-tmp.xml igraph-docs-with-resolved-includes.xml && /usr/local/bin/cmake -E remove igraph-docs-with-resolved-includes-tmp.xml
Traceback (most recent call last):
  File "/home/vsts/work/1/s/tools/removeexamples.py", line 37, in <module>
    main()
  File "/home/vsts/work/1/s/tools/removeexamples.py", line 24, in main
    tree.parse(sys.argv[1])
  File "/usr/lib/python3.10/xml/etree/ElementTree.py", line 580, in parse
    self._root = parser._parse_whole(source)
xml.etree.ElementTree.ParseError: undefined entity &nbsp;: line 55521, column 30

is telling me I have a non-breaking space somewhere (https://en.wikipedia.org/wiki/Non-breaking_space), but I'm not sure how to find where this problem is.

szhorvat · 2023-07-16T10:41:32Z

This is due to my change, I'll fix it in a moment.

Somehow the failure happens only on the CI but not locally.

szhorvat · 2023-07-16T10:47:28Z

Fixed on master.

szhorvat

Thanks! This is an initial review round, will take a more detailed look later.

szhorvat · 2023-07-28T12:26:28Z

src/paths/all_shortest_paths.c

                                  igraph_vector_int_list_t *vertices,
                                  igraph_vector_int_list_t *edges,
                                  igraph_vector_int_t *nrgeo,
                                  igraph_integer_t from, const igraph_vs_t to,
-                                  igraph_neimode_t mode) {
+                                  igraph_neimode_t mode, igraph_integer_t cutoff) {


Can we make the cutoff be an igraph_real_t even in the unweighted version, for clarity? This is the choice I made in igraph_distances(), with the reasoning that passing 2.1 should include paths of length 2.0. However, if the parameter is of integer type, then 2.1 is rounded to 2, and will exclude paths of length 2.

szhorvat · 2023-07-28T12:27:14Z

src/paths/all_shortest_paths.c

- * \function igraph_get_all_shortest_paths
- * \brief All shortest paths (geodesics) from a vertex.
+ * \function igraph_get_all_shortest_paths_cutoff
+ * \brief All shortest paths (geodesics) from a vertex, with cutoff.


Let's add \experimental for all new functions, even if changes are unlikely. We can remove that in 0.11. igraph_distances_cutoff() is also marked as experimental.

These docs say "added in 0.2", but that's for igraph_get_all_shortest_paths(), not for igraph_get_all_shortest_paths_cutoff().

szhorvat · 2023-07-28T12:28:20Z

src/paths/all_shortest_paths.c

+ * \function igraph_get_all_shortest_paths
+ * \brief All shortest paths (geodesics) from a vertex.
+ *
+ * \ref igraph_get_all_shortest_paths() without a cutoff.


Did you mean the following?

Suggested change

* \ref igraph_get_all_shortest_paths() without a cutoff.

* \ref igraph_get_all_shortest_paths_cutoff() without a cutoff.

Can we keep the full original docs here please, especially since the new function will be "experimental"?

szhorvat · 2023-07-28T12:30:05Z

src/paths/dijkstra.c

@@ -320,6 +320,31 @@ igraph_error_t igraph_shortest_paths_dijkstra(const igraph_t *graph,
 * \function igraph_get_shortest_paths_dijkstra
 * \brief Weighted shortest paths from a vertex.
 *
+ * \ref igraph_get_shortest_paths_dijkstra() without a cutoff.


Same comment as above, please keep the original docs, and add a reference to the cutoff version.

szhorvat · 2023-07-28T12:30:28Z

src/paths/dijkstra.c

+/**
+ * \ingroup structural
+ * \function igraph_get_shortest_paths_dijkstra_cutoff
+ * \brief Weighted shortest paths from a vertex with a cutoff.


Mark as experimental.

szhorvat · 2023-07-28T12:30:57Z

src/paths/dijkstra.c

@@ -705,8 +735,8 @@ igraph_error_t igraph_get_shortest_path_dijkstra(const igraph_t *graph,

 /**
 * \ingroup structural
- * \function igraph_get_all_shortest_paths_dijkstra
- * \brief All weighted shortest paths (geodesics) from a vertex.
+ * \function igraph_get_all_shortest_paths_dijkstra_cutoff


Mark as experimental.

szhorvat · 2023-07-28T12:31:20Z

src/paths/dijkstra.c

+
+/**
+ * \ingroup structural
+ * \function igraph_get_all_shortest_paths_dijkstra


Keep original docs.

szhorvat · 2023-07-28T12:31:35Z

src/paths/unweighted.c

- * \function igraph_get_shortest_paths
- * \brief Shortest paths from a vertex.
+ * \function igraph_get_shortest_paths_cutoff
+ * \brief Shortest paths from a vertex with a cutoff.


Mark as experimental.

szhorvat · 2023-07-28T12:31:48Z

src/paths/unweighted.c

                              igraph_vector_int_list_t *vertices,
                              igraph_vector_int_list_t *edges,
                              igraph_integer_t from, const igraph_vs_t to,
                              igraph_neimode_t mode,
                              igraph_vector_int_t *parents,
-                              igraph_vector_int_t *inbound_edges) {
+                              igraph_vector_int_t *inbound_edges,
+                              igraph_integer_t cutoff) {


Make it an igraph_real_t.

szhorvat · 2023-07-28T12:31:59Z

src/paths/unweighted.c

+/**
+ * \ingroup structural
+ * \function igraph_get_shortest_paths
+ * \brief Shortest paths from a vertex.


Keep original docs.

add get_shortest_paths_dijkstra_cutoff

3527a0c

GroteGnoom added 11 commits June 10, 2023 11:27

add igraph_get_shortest_paths_cutoff

debf6b9

add all shortest paths with cutoffs

ef4d1a8

check nrgeo for get_all_shortest_paths_dijkstra_cutoff

2af7966

add some docs

409421b

change paths cutoff docs to style used in distances

1fba965

add cutoff paths functions to functions.yaml

c5cfe37

Merge branch 'master' into shortest_paths_with_cutoff

c3b9a8a

add path cutoff functions to CHANGELOG.md

9f1fbca

shortest_path doc fixes

2fcaca3

add cutoff functions to docs xxml

a4007a2

fix underscore in igraph_get_shortest_paths_dijkstra_cutoff doc name

f4ef177

GroteGnoom added 3 commits July 16, 2023 19:11

Merge branch 'master' into shortest_paths_with_cutoff

a05ec8c

add all_shortest_paths_cutoff unweighted test where cutoff is used

eab5c89

fix typos

d7d6b7f

GroteGnoom marked this pull request as ready for review July 22, 2023 09:24

szhorvat requested changes Jul 28, 2023

View reviewed changes

szhorvat force-pushed the master branch from 41190cc to 9d06a76 Compare December 7, 2023 04:39

szhorvat force-pushed the master branch from 563d594 to 36a707a Compare May 23, 2024 13:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add cutoff versions of shortest path functions #2362

add cutoff versions of shortest path functions #2362

GroteGnoom commented Jun 10, 2023

codecov bot commented Jun 10, 2023 •

edited

GroteGnoom commented Jul 16, 2023

szhorvat commented Jul 16, 2023

szhorvat commented Jul 16, 2023

szhorvat left a comment

szhorvat Jul 28, 2023

szhorvat Jul 28, 2023

szhorvat Jul 28, 2023

szhorvat Jul 28, 2023

szhorvat Jul 28, 2023

szhorvat Jul 28, 2023

szhorvat Jul 28, 2023

szhorvat Jul 28, 2023

szhorvat Jul 28, 2023

szhorvat Jul 28, 2023

szhorvat Jul 28, 2023

	* \ref igraph_get_all_shortest_paths() without a cutoff.
	* \ref igraph_get_all_shortest_paths_cutoff() without a cutoff.

add cutoff versions of shortest path functions #2362

Are you sure you want to change the base?

add cutoff versions of shortest path functions #2362

Conversation

GroteGnoom commented Jun 10, 2023

codecov bot commented Jun 10, 2023 • edited

Codecov Report

GroteGnoom commented Jul 16, 2023

szhorvat commented Jul 16, 2023

szhorvat commented Jul 16, 2023

szhorvat left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Jun 10, 2023 •

edited