Conversation
| std::vector<Advisor> on_bounds; | ||
| std::vector<Advisor> on_array_size_change; | ||
|
|
||
| const dwave::optimization::ArrayNode* node_; |
There was a problem hiding this comment.
public now, no need for trailing _
|
|
||
| namespace dwave::optimization::cp { | ||
|
|
||
| struct BasicIndexingForwardTransform : IndexTransform { |
There was a problem hiding this comment.
Unless we need it public, we could move this within the implementation file, indexing_propagators.cpp
| } | ||
| } | ||
|
|
||
| TEST_CASE("BasicIndexingPropagator") { |
There was a problem hiding this comment.
To make things more organized, I moved the propagators tests within a subdir propagators/test_*.cpp, see #2 . We could do the same here.
|
|
||
| // build the advisor for the propagator p aimed to the variable for i | ||
| Advisor advisor_i(p, 0, std::make_unique<BasicIndexingForwardTransform>(i, b)); | ||
| vars[0]->propagate_on_domain_change(std::move(advisor_i)); |
There was a problem hiding this comment.
Ok, here you added this advisor for the basic indexing propagator to the domain_change list of of advisors. Which sounds correct, cause you can infer holes in the domains from the indexing node. However, your propagator implementation is only bound consistent - probably because I only added intervals for now and not other type of sets - so we should
| vars[0]->propagate_on_domain_change(std::move(advisor_i)); | |
| vars[0]->propagate_on_bounds_change(std::move(advisor_i)); |
There was a problem hiding this comment.
Also, as you can see in this PR for binaryop and reduce propagators I started adding this logic inside the constructor of the propagator.
|
|
||
| // build the advisor for the propagator p aimed to the variable for b | ||
| Advisor advisor_b(p, 1, std::make_unique<ElementWiseTransform>()); | ||
| vars[1]->propagate_on_domain_change(std::move(advisor_b)); |
There was a problem hiding this comment.
Like above:
| vars[1]->propagate_on_domain_change(std::move(advisor_b)); | |
| vars[1]->propagate_on_bounds_change(std::move(advisor_b)); |
| CPVarsState& v_state) const { | ||
| auto data = data_ptr<PropagatorData>(p_state); | ||
|
|
||
| const BasicIndexingNode* bi = dynamic_cast<const BasicIndexingNode*>(array_->node_); |
There was a problem hiding this comment.
If I understood things correctly, it should be
| const BasicIndexingNode* bi = dynamic_cast<const BasicIndexingNode*>(array_->node_); | |
| const BasicIndexingNode* bi = dynamic_cast<const BasicIndexingNode*>(basic_indexing_->node_); |
otherwise the dynamic cast is gonna fail in general..
| data->indices.to_process.pop_front(); | ||
| data->indices.is_scheduled[bi_index] = false; | ||
|
|
||
| std::vector<ssize_t> in_multi_index = |
There was a problem hiding this comment.
Maybe we can talk in person, but I'm getting confused with in and out here. Looks like your in is the output variable of the basic indexing node and and you out is the input of the basic indexing node.
wbernoudy
force-pushed
the
dev/cp-static-basic-indexing-propagator
branch
from
7fa3ae3 to
092ede7
Compare
|
Closing in favor of dwavesystems#501 |
2 participants
No description provided.