weightFunction to calculate weight of path ... #107

ravann · 2025-02-16T13:46:28Z

Created weightfunction to calculate weights of the path instead of hardcoded value ...

JesusTheHun · 2025-02-16T16:38:47Z

src/algorithms/shortestPath/getPath.ts

@@ -12,22 +19,25 @@ export function getPath<Node, LinkProps>(
  tracks: TraversingTracks<NoInfer<Node>>,
  source: NoInfer<Node>,
  destination: NoInfer<Node>,
+  weightFunction: (edgeWeight: number, currentPathWeight: number, hop: number) => number = addWeightFunction


I would call it "nextWeightFn", as "weightFunction" is more ambiguous

JesusTheHun · 2025-02-16T16:39:37Z

src/algorithms/shortestPath/getPath.ts

 ): {
  nodes: [Node, Node, ...Node[]];
  weight: number;
 } {
  const { p } = tracks;
  const nodeList: Node[] & { weight?: EdgeWeight } = [];

-  let totalWeight = 0;
+  let totalWeight = undefined as unknown as EdgeWeight;


Why cast the type to an invalid one instead of setting the correct type ?

JesusTheHun · 2025-02-16T16:40:36Z

src/algorithms/shortestPath/getPath.ts

@@ -12,22 +19,25 @@ export function getPath<Node, LinkProps>(
  tracks: TraversingTracks<NoInfer<Node>>,
  source: NoInfer<Node>,
  destination: NoInfer<Node>,
+  weightFunction: (edgeWeight: number, currentPathWeight: number, hop: number) => number = addWeightFunction


Prefer an object as a single parameter instead of multi parameters. It makes evolutions easier.

Also, make the properties more explicit + add jsdoc

curran

Great start! Happy to merge when:

New test cases are added that test the new behavior
Documentation is added to the README that explains the new option

* Added custom function document for shortestPath ... * Exporting WeightParams ... * Added WeightParams type ... * Changed weightFunction to use WeightParams ... * Changed the name of parameter ... * Added condition to check if the pathWeight is undefined ...

ravann · 2025-02-18T04:03:58Z

@JesusTheHun updated based on your comments. Please review.

@curran I updated the Readme.md. I could add tests on addWeightFunction, but its a simple function. Any guidance is welcome.

JesusTheHun · 2025-02-18T07:51:45Z

but its a simple function

Everything starts that way ^^
I agree with @curran, please write a test case for :

No given nextWeightFn, expect a sum of weights
Give a nextWeightFn, expect accordingly
Give a nextWeightFn, expect the correct parameters for a path with 3 nodes
Give a nextWeightFn to the shortest path function with a graph that contains at least 2 paths, for the sake of completion

JesusTheHun

Your work is appreciated !

JesusTheHun · 2025-02-16T18:42:53Z

src/algorithms/shortestPath/getPath.ts

@@ -12,22 +19,25 @@ export function getPath<Node, LinkProps>(
  tracks: TraversingTracks<NoInfer<Node>>,
  source: NoInfer<Node>,
  destination: NoInfer<Node>,
+  weightFunction: (edgeWeight: number, currentPathWeight: number, hop: number) => number = addWeightFunction


It's a good practice to make a reference to the graph accessible to the function

JesusTheHun · 2025-02-18T07:56:48Z

src/types.ts

@@ -18,3 +18,9 @@ export type SerializedInput<Node = unknown, LinkProps = unknown> = {
 };

 export type NoInfer<T> = [T][T extends any ? 0 : never];
+
+export type WeightParams = {


Maybe be more specific with NextWeightFnParams ?

This is done

Updated weight function name to NextWeightFnParams ... Added new unit tests to test the weight function related functionality

ravann · 2025-02-19T07:14:36Z

@JesusTheHun, @curran updated the code and Readme based on the changes suggested.

Btw, I realized getPath function traverses the graph in reverse order. It doesnt effect the default weight function OR my usecase. However, if there is a weight function that depends on how the graph is traversed, this can have an impact (e.g. newWeight = currentPathWeight + edgeWeight ** hop )

I don't have capacity to look into this right away, but can tackle this in a few weeks.

JesusTheHun · 2025-02-19T07:50:25Z

@ravann how do tests feel like now ? 😄

ravann · 2025-02-20T12:59:49Z

@curran is it ok to merge the PR?

JesusTheHun · 2025-02-20T13:29:37Z

src/types.ts

+  edgeWeight: EdgeWeight;
+  currentPathWeight: EdgeWeight | undefined;
+  hop: number;
+  sourceGraph: Graph<Node, LinkProps>;


This should be called "graph" to be consistent with other functions

JesusTheHun · 2025-02-20T13:34:55Z

src/types.ts

@@ -18,3 +20,10 @@ export type SerializedInput<Node = unknown, LinkProps = unknown> = {
 };

 export type NoInfer<T> = [T][T extends any ? 0 : never];
+
+export type NextWeightFnParams<Node = unknown, LinkProps = unknown> = {


I think we should also add the "path" as parameter, that would allow to get the total number of hops for the cases when the calculation is based on the distance from the source.

Edit: actually, I think we should also add the previous node and the current node, so the properties of the link can be retrieved from the graph.

This is done.

ravann · 2025-02-21T14:58:16Z

@JesusTheHun do you think the change can be merged now?

JesusTheHun · 2025-02-21T15:53:45Z

src/types.ts

+  currentPathWeight: EdgeWeight | undefined;
+  hop: number;
+  graph: Graph<Node, LinkProps>;
+  path: TraversingTracks<NoInfer<Node>>;


"NoInfer" only matters in a function signature.

curran

This is fantastic! I really appreciate the work on this @ravann and thank you so much @JesusTheHun for reviewing.

curran · 2025-02-21T19:31:24Z

Published in https://github.com/datavis-tech/graph-data-structure/releases/tag/v4.4.0

Ravann weightfunction (#1)

c8c56e7

Created weightfunction to calculate weights of the path instead of hardcoded value ...

ravann mentioned this pull request Feb 16, 2025

The weight calculation on the shortest path is not generic enough ... #106

Closed

JesusTheHun suggested changes Feb 16, 2025

View reviewed changes

curran requested changes Feb 17, 2025

View reviewed changes

JesusTheHun reviewed Feb 18, 2025

View reviewed changes

ravann added 2 commits February 19, 2025 14:06

Ravann patch 1 (#3)

0ba2777

Updated weight function name to NextWeightFnParams ... Added new unit tests to test the weight function related functionality

Added graph object to the NextWeightFnParams ...

6ea85cc

JesusTheHun suggested changes Feb 20, 2025

View reviewed changes

Add additional properties to the NextWeightFnParams ...

3113074

JesusTheHun approved these changes Feb 21, 2025

View reviewed changes

curran approved these changes Feb 21, 2025

View reviewed changes

curran merged commit 423d284 into datavis-tech:master Feb 21, 2025

JesusTheHun mentioned this pull request Feb 28, 2025

Calculate path weight by traversing from source to destination #109

Closed

weightFunction to calculate weight of path ... #107

weightFunction to calculate weight of path ... #107

Uh oh!

Conversation

ravann commented Feb 16, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

curran left a comment

Choose a reason for hiding this comment

Uh oh!

ravann commented Feb 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JesusTheHun commented Feb 18, 2025

Uh oh!

JesusTheHun left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ravann commented Feb 19, 2025

Uh oh!

JesusTheHun commented Feb 19, 2025

Uh oh!

ravann commented Feb 20, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JesusTheHun Feb 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ravann commented Feb 21, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

curran left a comment

Choose a reason for hiding this comment

Uh oh!

curran commented Feb 21, 2025

Uh oh!

Uh oh!

ravann commented Feb 18, 2025 •

edited

Loading

JesusTheHun Feb 20, 2025 •

edited

Loading