Skip to content

Fix issue with dijkstra algorithm#1497

Merged
jdumas merged 4 commits intolibigl:masterfrom
kekeblom:dijkstra_fix
May 14, 2020
Merged

Fix issue with dijkstra algorithm#1497
jdumas merged 4 commits intolibigl:masterfrom
kekeblom:dijkstra_fix

Conversation

@kekeblom
Copy link
Copy Markdown
Contributor

@kekeblom kekeblom commented Apr 28, 2020

On my system (gcc --version: g++ (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0), dijkstra doesn't work in the case below. It doesn't matter what the mesh is, you can test with your favorite one.

#include <igl/dijkstra.h>
#include <igl/read_triangle_mesh.h>
#include <igl/adjacency_list.h>
#include <iostream>
#include <Eigen/Core>
int main() {
  Eigen::MatrixXd V;
  Eigen::MatrixXi F;
  igl::read_triangle_mesh("gargoyle_cut.obj", V, F);


  std::vector<std::vector<int>> adj_list;
  igl::adjacency_list(F, adj_list, true);

  std::set<int> targets;
  int j = 200;
  targets.insert(j);
  Eigen::VectorXi min_distance, path;
  int success = igl::dijkstra(100, targets, adj_list, min_distance, path);

  std::cout << "success: " << success << " min_distance: " << min_distance[j] << std::endl;
}

std::numeric_limits<DerivedD::Scalar>::infinity() returns 0 in this case, which leads to the algorithm not finding the target and returning -1. Infinity probably only works with float values.

Before the change success would be -1. After the change, it returns the index for the target found and min_distance is correctly populated. Should probably add a unit test, but did not have time just now to figure out how to run the corresponding test.

Check all that apply (change to [x])

  • All changes meet libigl style-guidelines.
  • Adds new .cpp file.
  • Adds corresponding unit test.
  • This is a minor change.

@kekeblom
Copy link
Copy Markdown
Contributor Author

Added a test case.

@jdumas
Copy link
Copy Markdown
Collaborator

jdumas commented May 2, 2020

Seems it's missing some explicit template instantiation for the static lib version of libigl.

@kekeblom
Copy link
Copy Markdown
Contributor Author

That's seems to have done the trick.

@jdumas jdumas merged commit 1df0987 into libigl:master May 14, 2020
@jdumas
Copy link
Copy Markdown
Collaborator

jdumas commented May 14, 2020

Awesome, thanks!

skoch9 pushed a commit to skoch9/libigl that referenced this pull request Jun 3, 2020
* Use ::max vs ::infinity in dijkstra

* Add test case for discrete dijkstra distances

* Update dijkstra.cpp

* Explicit template instantiation for dijkstra test

Co-authored-by: Jérémie Dumas <jdumas@users.noreply.github.com>
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.

2 participants