Refactor matrix_action #4535
Refactor matrix_action #4535
Conversation
…also unify its signature; preparation for second pass on matrix
| kInitialEdgeLabelCountBidirDijkstra)), | ||
| clear_reserved_memory_(config.get<bool>("clear_reserved_memory", false)), |
There was a problem hiding this comment.
this is now in the base class
| const bool has_time, | ||
| const bool invariant, | ||
| const ShapeFormat& shape_format) { |
There was a problem hiding this comment.
I pulled all these out of the signature and set them inside the function instead. was pointless to have them in the signature, it'll all in the request, except for has_time which is being set via the base class's set_has_time()
| options.matrix_locations(), | ||
| options.date_time_type() == Options::invariant); | ||
| }; | ||
| MatrixAlgorithm* thor_worker_t::get_matrix_algorithm(Api& request, const bool has_time) { |
There was a problem hiding this comment.
the diff in here is impossible to read. will show tmrw in code editor what's going on there. the basic gist is:
- add a
get_matrix_algorithmfunction to thor_worker_t (like for routing), to get the properties of that class - remove the lambdas for the matrix algos and decide here which one to use
| &time_distance_matrix_, | ||
| &time_distance_bss_matrix_, | ||
| }) { | ||
| alg->set_interrupt(interrupt); |
There was a problem hiding this comment.
I added the interrupt function now for all matrix algos finally
| namespace { | ||
| static bool IsTrivial(const uint64_t& edgeid, | ||
| const valhalla::Location& origin, |
There was a problem hiding this comment.
this was duplicated with the exact same function in PathAlgorithm
| @@ -2,7 +2,7 @@ | |||
|
|
|||
| #include "baldr/json.h" | |||
| #include "proto_conversions.h" | |||
| #include "thor/matrix_common.h" | |||
There was a problem hiding this comment.
I also remove this header and pulled out what was there into the new header
| @@ -220,7 +220,7 @@ TEST(Matrix, test_matrix) { | |||
| CostMatrix cost_matrix; | |||
| cost_matrix.SourceToTarget(request, reader, mode_costing, sif::TravelMode::kDrive, 400000.0); | |||
| auto matrix = request.matrix(); | |||
| for (uint32_t i = 0; i < matrix.times().size(); ++i) { | |||
| for (int i = 0; i < matrix.times().size(); ++i) { | |||
There was a problem hiding this comment.
made this file warning-free
| * @param origin Origin path location information. | ||
| * @param destination Destination path location information. | ||
| */ | ||
| inline bool IsTrivial(const baldr::GraphId& edgeid, |
There was a problem hiding this comment.
this is changed from a member function to an inline function
|
Also, in a further step, we could think about somehow merging |
valhalla/thor/matrixalgorithm.h
Outdated
| // if `true` clean reserved memory for edge labels | ||
| bool clear_reserved_memory_; | ||
|
|
||
| // resizes all PBF sequences except for date_times |
nilsnolde
force-pushed
the
nn-matrix-refactor
branch
from
957a30b
to
d90c569
Compare
|
fixed the build @kevinkreiser , if you have a sec to re-approve |
as a preparation to #4474
similar to routing algos, I created a base class for matrix algos and unified the main public
SourceToTargetfunction. IMO the logic is a bit nicer now, but also it'll be much much easier to handle second passes this way.I should add this is really a pure refactor, no functional change is intended.