Constant DC verification #510

xJoskiy · 2025-01-06T19:06:07Z

Add DC verification with constant predicactes, add violations highlights and minimization algorithm

github-actions

clang-tidy made some suggestions

src/core/algorithms/dc/model/dc.cpp

src/core/algorithms/dc/parser/dc_parser.cpp

src/core/util/kdtree.h

github-actions

clang-tidy made some suggestions

src/core/algorithms/dc/verifier/dc_verifier.cpp

github-actions

clang-tidy made some suggestions

src/core/algorithms/dc/parser/dc_parser.cpp

github-actions

clang-tidy made some suggestions

src/core/algorithms/dc/model/column_operand.h

github-actions

clang-tidy made some suggestions

src/core/algorithms/dc/model/dc.cpp

src/core/algorithms/dc/parser/dc_parser.cpp

github-actions

clang-tidy made some suggestions

src/core/util/kdtree.h

src/core/algorithms/dc/model/column_operand.h

BUYT-1 · 2025-02-20T07:54:13Z

src/python_bindings/dc/bind_dc_verification.cpp


-}  // namespace python_bindings
+    py::bind_vector<std::vector<std::pair<size_t, size_t>>>(main_module, "vector_pair");


Is there an issue with the standard conversion?

BUYT-1 · 2025-02-20T07:55:50Z

src/core/algorithms/dc/model/column_operand.h

 //  @brief Represents a column operand for Predicate.
 //
 //  A predicate (e.g., t.A == s.A) comprises three elements:
 //  the column operand from the first tuple ("t.A"), the comparison operator
 //  ("=="), and the column operand from the second tuple ("s.A"). The `ColumnOperand` class
 //  encapsulates the column operand part of a predicate, such as "t.A" or "s.A".
 //
+//  A constant value also can be a column operand thus boost::optional is utilized
+//
 //  The class distinguishes between operands derived from the first tuple (t) and those


Looks outdated

Does not seem to be resolved, I still see the comment about is_first_tuple_

I've only resolved for my convenience. I haven't yet uploaded a new version that fixes this issue thus didn't ask for re-review. I think this is a common practice

Oh, yeah, my bad, sorry :)

ol-imorozko · 2025-02-24T16:37:45Z

src/core/algorithms/dc/model/column_operand.h

 //  @brief Represents a column operand for Predicate.
 //
 //  A predicate (e.g., t.A == s.A) comprises three elements:
 //  the column operand from the first tuple ("t.A"), the comparison operator
 //  ("=="), and the column operand from the second tuple ("s.A"). The `ColumnOperand` class
 //  encapsulates the column operand part of a predicate, such as "t.A" or "s.A".
 //
+//  A constant value also can be a column operand thus boost::optional is utilized


Suggested change

// A constant value also can be a column operand thus boost::optional is utilized

// A constant value also can be a column operand thus std::optional is utilized

ol-imorozko · 2025-02-24T16:40:35Z

src/core/algorithms/dc/model/column_operand.h

 //  @brief Represents a column operand for Predicate.
 //
 //  A predicate (e.g., t.A == s.A) comprises three elements:
 //  the column operand from the first tuple ("t.A"), the comparison operator
 //  ("=="), and the column operand from the second tuple ("s.A"). The `ColumnOperand` class
 //  encapsulates the column operand part of a predicate, such as "t.A" or "s.A".
 //
+//  A constant value also can be a column operand thus boost::optional is utilized
+//
 //  The class distinguishes between operands derived from the first tuple (t) and those


Does not seem to be resolved, I still see the comment about is_first_tuple_

ol-imorozko · 2025-02-24T16:44:40Z

src/core/algorithms/dc/model/column_operand.h

+enum class Tuple { kS, kT, kMixed };
+
 //  @brief Represents a column operand for Predicate.
 //
 //  A predicate (e.g., t.A == s.A) comprises three elements:
 //  the column operand from the first tuple ("t.A"), the comparison operator
 //  ("=="), and the column operand from the second tuple ("s.A"). The `ColumnOperand` class
 //  encapsulates the column operand part of a predicate, such as "t.A" or "s.A".


It's hard to me to grasp what kMixed represents here. I suggest you to add the explanation to this comment right here that explains what "t" and "s" means

kMixed is used for defining a tuple on Predicate since it may have ColumnPredicate's with kS and sT tuples. We could also create two enums, for Predicate and ColumnPredicate but I'm not sure, couldn't come up with other solution.

ol-imorozko · 2025-02-24T16:47:22Z

src/core/algorithms/dc/model/column_operand.h

+        std::byte* val = type->Allocate();
+        type->ValueFromStr(val, str_val);


Feel free to ignore:
I would use type_ here. Makes more sense to me that in constructor we filled some private fields first and then called methods on those fields, instead on what we filled them with (though it's the same pointer, naturally)

ol-imorozko · 2025-02-24T16:54:13Z

src/core/algorithms/dc/model/column_operand.h

+    bool operator!=(ColumnOperand const& rhs) const {
        return !(*this == rhs);
    }


I think C++20 can do this automatically with bool operator!=(ColumnOperand const& rhs) = default assuming operator== is provided

ol-imorozko · 2025-02-24T16:57:16Z

src/core/algorithms/dc/model/column_operand.h

 //  The class distinguishes between operands derived from the first tuple (t) and those
 //  from the second tuple (s) using a boolean flag `is_first_tuple_`, where `true` indicates an
 //  operand from the first tuple (t), and `false` indicates an operand from the second
 //  tuple (s).
 class ColumnOperand {
 private:
    Column const* column_;
-    bool is_first_tuple_;
+    std::optional<dc::Tuple> tuple_;


what does std::optional here achieve?
I might be wrong, but I think the only real usage of a tuple_ is within a GetTuple method which assert that it has value anyway.

Actually, a ColumnOperand may have a tuple_ variable initialized or may not. If a ColumnOperand is a constant value, then tuple_ is not initialized. In this case accessing tuple_ is not allowed. I should definitely write it above

Now it is possible to pass an option that tells the algorithm whether to collect violations or not.

github-actions bot reviewed Jan 6, 2025

View reviewed changes

xJoskiy force-pushed the DC-verification branch 2 times, most recently from cee6cc2 to 02b27b6 Compare January 14, 2025 00:54

github-actions bot reviewed Jan 14, 2025

View reviewed changes

src/core/algorithms/dc/verifier/dc_verifier.cpp Outdated Show resolved Hide resolved

src/core/algorithms/dc/verifier/dc_verifier.cpp Outdated Show resolved Hide resolved

src/core/algorithms/dc/verifier/dc_verifier.cpp Outdated Show resolved Hide resolved

xJoskiy force-pushed the DC-verification branch from 02b27b6 to 5e7dbbc Compare January 14, 2025 18:09

github-actions bot reviewed Jan 14, 2025

View reviewed changes

src/core/algorithms/dc/parser/dc_parser.cpp Outdated Show resolved Hide resolved

src/core/algorithms/dc/parser/dc_parser.cpp Outdated Show resolved Hide resolved

xJoskiy force-pushed the DC-verification branch 2 times, most recently from ebd4eae to deaa199 Compare January 20, 2025 20:21

xJoskiy changed the title ~~WIP: Constant DC verification~~ Constant DC verification Jan 20, 2025

github-actions bot reviewed Jan 20, 2025

View reviewed changes

src/core/algorithms/dc/model/column_operand.h Outdated Show resolved Hide resolved

xJoskiy force-pushed the DC-verification branch 3 times, most recently from a90e568 to 3d0b351 Compare January 25, 2025 10:07

github-actions bot reviewed Jan 25, 2025

View reviewed changes

src/core/algorithms/dc/model/dc.cpp Outdated Show resolved Hide resolved

src/core/algorithms/dc/parser/dc_parser.cpp Outdated Show resolved Hide resolved

src/core/algorithms/dc/parser/dc_parser.cpp Outdated Show resolved Hide resolved

xJoskiy force-pushed the DC-verification branch 5 times, most recently from ce333d9 to 9e2a146 Compare January 27, 2025 05:37

xJoskiy force-pushed the DC-verification branch from 9e2a146 to f805864 Compare February 3, 2025 13:47

github-actions bot reviewed Feb 3, 2025

View reviewed changes

src/core/util/kdtree.h Outdated Show resolved Hide resolved

src/core/util/kdtree.h Outdated Show resolved Hide resolved

xJoskiy force-pushed the DC-verification branch from f805864 to 992bb4b Compare February 7, 2025 20:32

BUYT-1 reviewed Feb 20, 2025

View reviewed changes

src/core/algorithms/dc/model/column_operand.h Outdated Show resolved Hide resolved

BUYT-1 reviewed Feb 20, 2025

View reviewed changes

ol-imorozko requested changes Feb 24, 2025

View reviewed changes

xJoskiy added 4 commits February 24, 2025 21:40

Add DC verifier highlights

6f6de3c

Add DC parser that allows constants

66c4d90

Reorganize checking methods for dc

c88dc25

Add constant DC verification support

b7c569f

xJoskiy added 5 commits February 24, 2025 23:08

Add string represantation of a rectangle

0ebbf4c

Add data cleaning scenario

27f992a

Add do_collect_violations option

7a76d5a

Now it is possible to pass an option that tells the algorithm whether to collect violations or not.

Add tuple enum

26c9d34

Fix DCVerifier bindings

240a539

xJoskiy force-pushed the DC-verification branch from 16a6c92 to 240a539 Compare February 25, 2025 11:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Constant DC verification #510

Constant DC verification #510

xJoskiy commented Jan 6, 2025

github-actions bot left a comment

github-actions bot left a comment

github-actions bot left a comment

github-actions bot left a comment

github-actions bot left a comment

github-actions bot left a comment

BUYT-1 Feb 20, 2025

BUYT-1 Feb 20, 2025

ol-imorozko Feb 24, 2025

xJoskiy Feb 24, 2025

ol-imorozko Feb 24, 2025

ol-imorozko Feb 24, 2025

ol-imorozko Feb 24, 2025

ol-imorozko Feb 24, 2025

xJoskiy Feb 24, 2025

ol-imorozko Feb 24, 2025

ol-imorozko Feb 24, 2025

ol-imorozko Feb 24, 2025

xJoskiy Feb 24, 2025 •

edited

Loading


		} // namespace python_bindings
		py::bind_vector<std::vector<std::pair<size_t, size_t>>>(main_module, "vector_pair");

	// A constant value also can be a column operand thus boost::optional is utilized
	// A constant value also can be a column operand thus std::optional is utilized

		std::byte* val = type->Allocate();
		type->ValueFromStr(val, str_val);

Constant DC verification #510

Are you sure you want to change the base?

Constant DC verification #510

Conversation

xJoskiy commented Jan 6, 2025

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xJoskiy Feb 24, 2025 • edited Loading

Choose a reason for hiding this comment

xJoskiy Feb 24, 2025 •

edited

Loading