change kdTree code to choose the number of leaves #962
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
julien-tierny
requested changes
Aug 31, 2023
core/base/kdTree/KDTree.h
Outdated
| @@ -64,14 +64,18 @@ namespace ttk { | |||
| const int &ptNumber, | |||
| const int &dimension, | |||
| const std::vector<std::vector<dataType>> &weights = {}, | |||
| const int weight_number = 1); | |||
| const int weight_number = 1, | |||
There was a problem hiding this comment.
I'd recommend to take the habit to use references for function parameters (unless pointers are necessary), even for integers.
const int weight_number = 1 -> const int &weightNumber = 1
(also, we usually don't use underscores for names in TTK, but a camel wording)
core/base/kdTree/KDTree.h
Outdated
| @@ -64,14 +64,18 @@ namespace ttk { | |||
| const int &ptNumber, | |||
| const int &dimension, | |||
| const std::vector<std::vector<dataType>> &weights = {}, | |||
| const int weight_number = 1); | |||
| const int weight_number = 1, | |||
| int nodeNumber = -1); | |||
There was a problem hiding this comment.
int &nodeNumber = -1 --> const int &nodeNumber = -1 (unless it is not read only)
|
|
||
| void buildRecursive(dataType *data, | ||
| std::vector<int> &idx_side, | ||
| const int &ptNumber, | ||
| const int &dimension, | ||
| KDTree<dataType, Container> *parent, | ||
| KDTreeMap &correspondence_map, | ||
| const int nodeNumber, | ||
| const int maximumLevel, | ||
| int &createdNumberNode, |
There was a problem hiding this comment.
same as above const (if indeed read only)
core/base/kdTree/KDTree.h
Outdated
| @@ -146,15 +150,67 @@ typename ttk::KDTree<dataType, Container>::KDTreeMap | |||
| const int &ptNumber, | |||
| const int &dimension, | |||
| const std::vector<std::vector<dataType>> &weights, | |||
| const int weight_number) { | |||
| const int weight_number, | |||
| int nodeNumber) { | |||
There was a problem hiding this comment.
same remark as above const int &nodeNumber
core/base/kdTree/KDTree.h
Outdated
| @@ -146,15 +150,67 @@ typename ttk::KDTree<dataType, Container>::KDTreeMap | |||
| const int &ptNumber, | |||
| const int &dimension, | |||
| const std::vector<std::vector<dataType>> &weights, | |||
| const int weight_number) { | |||
| const int weight_number, | |||
There was a problem hiding this comment.
same remark as above const int &weightNumber.
core/base/kdTree/KDTree.h
Outdated
| @@ -225,23 +291,35 @@ void ttk::KDTree<dataType, Container>::buildRecursive( | |||
| const int &dimension, | |||
| KDTree<dataType, Container> *parent, | |||
| KDTreeMap &correspondence_map, | |||
| const int nodeNumber, | |||
There was a problem hiding this comment.
const int &nodeNumber
core/base/kdTree/KDTree.h
Outdated
| @@ -225,23 +291,35 @@ void ttk::KDTree<dataType, Container>::buildRecursive( | |||
| const int &dimension, | |||
| KDTree<dataType, Container> *parent, | |||
| KDTreeMap &correspondence_map, | |||
| const int nodeNumber, | |||
| const int maximumLevel, | |||
There was a problem hiding this comment.
const int &maximumLevel
| @@ -225,23 +291,35 @@ void ttk::KDTree<dataType, Container>::buildRecursive( | |||
| const int &dimension, | |||
| KDTree<dataType, Container> *parent, | |||
| KDTreeMap &correspondence_map, | |||
| const int nodeNumber, | |||
| const int maximumLevel, | |||
| int &createdNumberNode, | |||
There was a problem hiding this comment.
const int &createdNumberNode (unless not read-only)
core/base/kdTree/KDTree.h
Outdated
| @@ -288,10 +368,13 @@ void ttk::KDTree<dataType, Container>::buildRecursive( | |||
| this->left_ | |||
| = std::make_unique<KDTree>(this, (coords_number_ + 1) % dimension, true); | |||
| this->left_->buildRecursive(data, idx_left, ptNumber, dimension, this, | |||
| correspondence_map, weights, weight_number); | |||
| correspondence_map, nodeNumber, maximumLevel, | |||
| createdNumberNode, weights, weight_number); | |||
core/base/kdTree/KDTree.h
Outdated
| @@ -300,7 +383,8 @@ void ttk::KDTree<dataType, Container>::buildRecursive( | |||
| this->right_ | |||
| = std::make_unique<KDTree>(this, (coords_number_ + 1) % dimension, false); | |||
| this->right_->buildRecursive(data, idx_right, ptNumber, dimension, this, | |||
| correspondence_map, weights, weight_number); | |||
| correspondence_map, nodeNumber, maximumLevel, | |||
| createdNumberNode, weights, weight_number); | |||
|
thanks a lot, let's go! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Thanks for contributing to TTK!
Before submitting your pull request, please:
Review our Contributor Guidelines, in particular regarding code formatting (with clang-format) and continuous integration.
Please provide a quick description of your contributions below:
Your description here