graphtools icon indicating copy to clipboard operation
graphtools copied to clipboard

Patches: tasklogger `log_x`, `randomized_svd` arguments, deprecated `graph_shortest_path`

Open stanleyjs opened this issue 4 years ago • 0 comments

Good day,

This PR began its life to fix the issues with randomized svd discussed in #58. To fix that, I monkey patched in a version of randomized svd using mock that respects arguments passed in the dataclass bipca.base.PCAParameters. The patch works on all versions of sklearn, though it raises a warning to the user to let the maintainers of graphtools know that sklearn has been updated when the version exceeds the current version. The PCAParameters dataclass has been documented and the pca_params kwarg to graphtools.base.Data has been documented.

I also made some small changes that made the tests run smoothly. I changed all calls to tasklogger.debug,.task,.info to their new log_x version. I changed instances of sklearn.utils.graph_shortest_path.graph_shortest_path to scipy.sparse.csgraph.shortest_path, reflecting the fact that the former has been deprecated.

The monkey patch required changing the way that SVD is done in the tests to make things line up properly. I fixed this in test_exact.test_truncated_exact_graph_sparse and test_knn.test_knn_graph_sparse.

This build currently fails two tests reliably, as documented in #60. This can be fixed pretty quickly using the suggested changes in that issue. This build intermittently fails test_exact.test_shortest_path_affinity as discussed in #61. I believe master fails this as well.

stanleyjs avatar Jan 03 '22 18:01 stanleyjs