-
Notifications
You must be signed in to change notification settings - Fork 66
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
Optimize hymd #386
base: main
Are you sure you want to change the base?
Optimize hymd #386
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-tidy made some suggestions
assert(!NotEmpty(cur_node.rhs_bounds)); | ||
assert(cur_node_index > md.lhs_specialization.specialized.index); | ||
auto const& [rhs_index, rhs_bound] = md.rhs; | ||
auto set_bound = [&](MdNode* node) { node->rhs_bounds[rhs_index] = rhs_bound; }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: Dereference of undefined pointer value [clang-analyzer-core.NullDereference]
auto set_bound = [&](MdNode* node) { node->rhs_bounds[rhs_index] = rhs_bound; };
^
Additional context
src/core/algorithms/md/hymd/lattice/md_lattice.cpp:174: '?' condition is true
assert(!NotEmpty(cur_node.rhs_bounds));
^
/usr/include/assert.h:92: expanded from macro 'assert'
(static_cast <bool> (expr) \
^
src/core/algorithms/md/hymd/lattice/md_lattice.cpp:175: Assuming 'cur_node_index' is > field 'index'
assert(cur_node_index > md.lhs_specialization.specialized.index);
^
/usr/include/assert.h:92: expanded from macro 'assert'
(static_cast <bool> (expr) \
^
src/core/algorithms/md/hymd/lattice/md_lattice.cpp:175: '?' condition is true
assert(cur_node_index > md.lhs_specialization.specialized.index);
^
/usr/include/assert.h:92: expanded from macro 'assert'
(static_cast <bool> (expr) \
^
src/core/algorithms/md/hymd/lattice/md_lattice.cpp:178: Calling 'AddUnchecked<algos::hymd::lattice::MdLattice::MdNode, (lambda at /github/workspace/src/core/algorithms/md/hymd/lattice/md_lattice.cpp:178:22)>'
AddUnchecked(&cur_node, md.lhs_specialization.old_lhs, cur_node_index, set_bound);
^
src/core/algorithms/md/hymd/lattice/node_base.h:70: '?' condition is true
assert(cur_node_ptr->IsEmpty());
^
/usr/include/assert.h:92: expanded from macro 'assert'
(static_cast <bool> (expr) \
^
src/core/algorithms/md/hymd/lattice/node_base.h:71: Loop condition is false. Execution continues on line 79
for (MdElement element = lhs.FindNextNonZero(cur_node_index); lhs.IsNotEnd(element);
^
src/core/algorithms/md/hymd/lattice/node_base.h:78: Calling 'operator()'
final_node_action(cur_node_ptr);
^
src/core/algorithms/md/hymd/lattice/md_lattice.cpp:177: Dereference of undefined pointer value
auto set_bound = [&](MdNode* node) { node->rhs_bounds[rhs_index] = rhs_bound; };
^
src/core/algorithms/md/hymd/md_lhs.h
Outdated
template <> | ||
struct hash<algos::hymd::MdLhs> { | ||
std::size_t operator()(algos::hymd::MdLhs const& p) const noexcept { | ||
return hash<algos::hymd::DecisionBoundaryVector>{}(p.GetValues()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: no member named 'DecisionBoundaryVector' in namespace 'algos::hymd' [clang-diagnostic-error]
return hash<algos::hymd::DecisionBoundaryVector>{}(p.GetValues());
^
NewBounds new_bounds_; | ||
|
||
public: | ||
class update_iterator { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: invalid case style for class 'update_iterator' [readability-identifier-naming]
class update_iterator { | |
class UpdateIterator { |
src/core/algorithms/md/hymd/utility/invalidated_rhss.h:25:
- update_iterator(InvalidatedIterator invalidated_iter, NewBoundsIterator new_bounds_iter)
+ UpdateIterator(InvalidatedIterator invalidated_iter, NewBoundsIterator new_bounds_iter)
src/core/algorithms/md/hymd/utility/invalidated_rhss.h:35:
- update_iterator operator++() {
+ UpdateIterator operator++() {
src/core/algorithms/md/hymd/utility/invalidated_rhss.h:41:
- update_iterator operator++(int) {
- update_iterator old = *this;
+ UpdateIterator operator++(int) {
+ UpdateIterator old = *this;
src/core/algorithms/md/hymd/utility/invalidated_rhss.h:47:
- friend bool operator==(update_iterator const& iter1, update_iterator const& iter2) {
+ friend bool operator==(UpdateIterator const& iter1, UpdateIterator const& iter2) {
src/core/algorithms/md/hymd/utility/invalidated_rhss.h:51:
- friend bool operator!=(update_iterator const& iter1, update_iterator const& iter2) {
+ friend bool operator!=(UpdateIterator const& iter1, UpdateIterator const& iter2) {
src/core/algorithms/md/hymd/utility/invalidated_rhss.h:67:
- update_iterator UpdateIterBegin() const {
+ UpdateIterator UpdateIterBegin() const {
src/core/algorithms/md/hymd/utility/invalidated_rhss.h:71:
- update_iterator UpdateIterEnd() const {
+ UpdateIterator UpdateIterEnd() const {
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-tidy made some suggestions
assert(!NotEmpty(cur_node.rhs_bounds)); | ||
assert(cur_node_index > md.lhs_specialization.specialized.index); | ||
auto const& [rhs_index, rhs_bound] = md.rhs; | ||
auto set_bound = [&](MdNode* node) { node->rhs_bounds[rhs_index] = rhs_bound; }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: Dereference of undefined pointer value [clang-analyzer-core.NullDereference]
auto set_bound = [&](MdNode* node) { node->rhs_bounds[rhs_index] = rhs_bound; };
^
Additional context
src/core/algorithms/md/hymd/lattice/md_lattice.cpp:182: '?' condition is true
assert(!NotEmpty(cur_node.rhs_bounds));
^
/usr/include/assert.h:92: expanded from macro 'assert'
(static_cast <bool> (expr) \
^
src/core/algorithms/md/hymd/lattice/md_lattice.cpp:183: Assuming 'cur_node_index' is > field 'index'
assert(cur_node_index > md.lhs_specialization.specialized.index);
^
/usr/include/assert.h:92: expanded from macro 'assert'
(static_cast <bool> (expr) \
^
src/core/algorithms/md/hymd/lattice/md_lattice.cpp:183: '?' condition is true
assert(cur_node_index > md.lhs_specialization.specialized.index);
^
/usr/include/assert.h:92: expanded from macro 'assert'
(static_cast <bool> (expr) \
^
src/core/algorithms/md/hymd/lattice/md_lattice.cpp:186: Calling 'AddUnchecked<algos::hymd::lattice::MdNode, (lambda at /github/workspace/src/core/algorithms/md/hymd/lattice/md_lattice.cpp:186:22)>'
AddUnchecked(&cur_node, md.lhs_specialization.old_lhs, cur_node_index, set_bound);
^
src/core/algorithms/md/hymd/lattice/node_base.h:72: '?' condition is true
assert(cur_node_ptr->IsEmpty());
^
/usr/include/assert.h:92: expanded from macro 'assert'
(static_cast <bool> (expr) \
^
src/core/algorithms/md/hymd/lattice/node_base.h:73: Loop condition is false. Execution continues on line 81
for (MdElement element = lhs.FindNextNonZero(cur_node_index); lhs.IsNotEnd(element);
^
src/core/algorithms/md/hymd/lattice/node_base.h:80: Calling 'operator()'
final_node_action(cur_node_ptr);
^
src/core/algorithms/md/hymd/lattice/md_lattice.cpp:185: Dereference of undefined pointer value
auto set_bound = [&](MdNode* node) { node->rhs_bounds[rhs_index] = rhs_bound; };
^
This thread pool is designed for the use case where there are many batches of similar tasks. The thread pool can be waited on without requiring the creation of threads again, and in each batch resources can be shared between tasks that happen to be done on a single thread. For example, we may only allocate a buffer once per batch per thread, or even fewer times if tasks in separated batches are the same, instead of allocating it for every single task.
Don't check MDs with LHSs that generalize the LHS of the MD that was removed before specialization.
LHS is now represented as an array of (offset, decision boundary) pairs instead of an array of decision boundaries.
Decreases runtime for datasets where inference from record pairs is the optimal algorithm.
Don't calculate the metric if the length difference between the strings is too great.
No description provided.