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

Stall on demand #19

Closed

Conversation

Stunkymonkey
Copy link
Contributor

@Stunkymonkey Stunkymonkey commented Apr 14, 2020

yes it runs fine, but I still do not see any speedup.

PS: this PR is not on to master

@Stunkymonkey
Copy link
Contributor Author

please do performance tests yourself

@Stunkymonkey
Copy link
Contributor Author

@easbar is there anything I can help you with?

Copy link
Owner

@easbar easbar left a comment

Choose a reason for hiding this comment

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

I did not have a look at this because you said there was no speedup: #18 (comment)

Now I did some quick runs and it seems like there might be a 5-10% improvement, so still not sure if this is worth it tbh.

Comment on lines -180 to -181
let ranks_copy = self.fast_graph.ranks.clone();
for i in 0..ranks_copy.len() {
Copy link
Owner

Choose a reason for hiding this comment

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

Nice we do not have to do this. This is unrelated to stall on demand though, right? Can you open a separate PR for this to master? Otherwise I will just commit this straight to master.

Copy link
Owner

Choose a reason for hiding this comment

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

I commited it to master here: 192ae19

@@ -173,7 +173,10 @@ impl PathCalculator {
let adj = graph.edges_bwd[edge_id].adj_node;
let edge_weight = graph.edges_bwd[edge_id].weight;
let adj_weight = self.get_weight_fwd(adj);
if adj_weight != WEIGHT_MAX && adj_weight + edge_weight < curr.weight {
if graph.ranks[adj] > graph.ranks[curr.node_id]
Copy link
Owner

Choose a reason for hiding this comment

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

I still do not see why we need to check the ranks here. Do you have an example where no path is found without this check as you said here: #18 (comment)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants