Skip to content
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

[SetTheoretic] Add SetTheoretic concept #364

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

shreyans800755
Copy link
Contributor

It adds SetTheoretic concept for functions-
union, intersection, difference, symmetric_difference.

Closes #357

@shreyans800755
Copy link
Contributor Author

@ldionne Can you please review this ?

Copy link
Member

@ldionne ldionne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great start, and thanks a lot for the work you're putting into it!

include/boost/hana/fwd/concept/set_theoretic.hpp Outdated Show resolved Hide resolved
BOOST_HANA_NAMESPACE_BEGIN
//! @ingroup group-concepts
//! @defgroup group-SetTheoretic SetTheoretic
//! The `SetTheoretic` concept represents data structures supporting
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Defining the concept as "things that have the methods that the concept provides" is kind of circular, and not very useful IMO. Also, you're not defining laws for the concept. I'd go for a brief like this instead:

The SetTheoretic concept represents data structures that support an algebra of sets.

Then, I'd make some comments about what the concept is at a high level, and why it's useful. I'd also give the laws that the concept must follow, e.g. probably the ones on the Wikipedia page.

Other comments:

  • One thing that must be thought about is the relationship between this concept and Searchable. Indeed, Searchable provides is_subset; it seems clear that there's an interesting mathematical link between both, and it should be explored, explained and some laws should be surfaced for data structures that are both Searchable and SetTheoretic.
  • You should also think about automatically-provided models and document them, if you can find some. What I mean here is that you should try to see whether satisfying some other concept(s) makes it possible to satisfy SetTheoretic automatically. When this is the case, Hana usually provides such a definition and documents it (e.g. Foldable for any Iterable).
  • The methods should be documented like normal methods that are tied to a concept, not individually for map and set.

I know this may seem like a lot, but this is what a concept in Hana is; it's not only a syntactical construction, it's also a tool to reason mathematically about programs, which requires a little more work to define than "loose" concepts that don't have clear semantics, if I may put it that way.

If there are things where you really don't know what to do, let me know and I'll try to make some time for researching the topic to try and provide some guidance.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense for SetTheoretic to have its own accessors? Something like get<T>(set) == at_key(set, T{}) would be very useful for set and it would be a way of making SetTheoretic provide an implementation for Searchable.

It might even simplify implementation of these functions assuming "keys" are strictly looked up at compile-time. Hmm.. that might also require something like the keys function that map has.

Also perhaps it should be just hana::Set.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The TLDR is that they would only have to implement those accessors and not the union, intersection,... algorithms directly

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I see it does not necessarily make sense to have SetTheoretic be related to Searchable if the goal is to create a generic thing that supports the algebra of sets.

You can make plenty of things that support the algebra of sets that it does not make sense to be Searchable.

For example: a string of bits:

1010 union 0001 => 1011
1010 intersection 1000 => 1000
1010 difference 1000 => 0010
1010 symmetric_difference 1001 => 0011

Anyways......thats just the way I see it.......
@ldionne @ricejasonf @shreyans800755

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DarthRubik I don't believe we are trying to couple Set with Searchable, but types that satisfy Searchable can be made to implement the needed functions for Set. For your string of bits you would probably implement those functions directly.

include/boost/hana/intersection.hpp Outdated Show resolved Hide resolved
example/misc/infinite_set.cpp Show resolved Hide resolved
include/boost/hana/map.hpp Outdated Show resolved Hide resolved
include/boost/hana/set.hpp Outdated Show resolved Hide resolved
include/boost/hana/symmetric_difference.hpp Outdated Show resolved Hide resolved
include/boost/hana/union.hpp Outdated Show resolved Hide resolved
include/boost/hana/difference.hpp Outdated Show resolved Hide resolved
@shreyans800755
Copy link
Contributor Author

Resuming work after ages. Would like complete it this time.

@shreyans800755 shreyans800755 force-pushed the concepts branch 4 times, most recently from b6c6a74 to c2f9e0b Compare February 21, 2020 00:33
@@ -56,6 +62,22 @@ namespace boost { namespace hana {
}
};

template <>
struct difference_impl<infinite_set_tag> {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ldionne
This actually doesn't seem good to me logically. Although infinite set had union function implemented, we can't call it, because apparantly, union requires the class to be SetTheoretic meaning it requires minimal definition - all 3 functions union, intersection & difference. But, mathematically, it seems accurate though.
Any thoughts ?

It adds SetTheoretic concept for functions-
union, intersection, difference, symmetric_difference.

Closes boostorg#357
@shreyans800755
Copy link
Contributor Author

Also, mathematically, this doesn't seem accurate. Because, ideally intersection must be commutative, but here only set::intersection is commutative, but map::intersection is not.
@ldionne I'm not sure if this was intended, but your input would be appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants