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

new factory for algebra of sets #1251

Merged
merged 3 commits into from
Jul 31, 2024

Conversation

affeldt-aist
Copy link
Member

Co-authored-by: @JeremyDubut
Co-authored-by: @AkihisaYamada

Motivation for this change
Checklist
  • added corresponding entries in CHANGELOG_UNRELEASED.md

- [ ] added corresponding documentation in the headers

Reference: How to document

Reminder to reviewers

@affeldt-aist affeldt-aist added the enhancement ✨ This issue/PR is about adding new features enhancing the library label Jun 26, 2024
@affeldt-aist affeldt-aist added this to the 1.3.0 milestone Jun 26, 2024
@affeldt-aist
Copy link
Member Author

affeldt-aist commented Jul 9, 2024

NB: The main issue is to find a better name instead of isAlgebraOfSetsD renamed

@affeldt-aist
Copy link
Member Author

Despite the fact that symmetric difference is not yet exploited seriously, this PR already went through several eyes and since there is no disagreement we might as well merge it soon.

@affeldt-aist
Copy link
Member Author

@t6s ping ?

Copy link
Member

@t6s t6s left a comment

Choose a reason for hiding this comment

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

Adding the symmetric difference will be useful, but the choice of laws looks a bit random. What about calling the symmetric difference "add?", ? being some appropriate letter for sets, and provide the ring laws? (mul = setI, opp = idfun)

theories/measure.v Outdated Show resolved Hide resolved
@t6s
Copy link
Member

t6s commented Jul 26, 2024

setdiffA, setdiffC, set0diff, setdiff0, set1diff, setdiff1, setdiffBAC, etc. might be better? But I am not sure how to handle names like mul?Dl .

affeldt-aist and others added 2 commits July 29, 2024 15:22
Co-authored-by: Takafumi Saikawa <[email protected]>
@affeldt-aist
Copy link
Member Author

affeldt-aist commented Jul 29, 2024

What about calling the symmetric difference "add?", ? being some appropriate letter for sets, and provide the ring laws? (mul = setI, opp = idfun)

what about setX for xor instead of sym_diff/add? sounds even more a propos

@affeldt-aist
Copy link
Member Author

affeldt-aist commented Jul 29, 2024

What about the last commit 9ef13c2 ?

sym_diff becomes setX for XOR (setX was not taken, the cartesian product is setM).
(It is true that add could be used instead of xor but setD is already taken by set difference...)
It also provides instantiations of commutative monoid and ring laws.

@t6s
Copy link
Member

t6s commented Jul 29, 2024

Looks good. The conflict with finset in the naming for direct products is fearsome, but I believe that that is already discussed when setM was introduced.

@t6s
Copy link
Member

t6s commented Jul 29, 2024

I cannot figure out how to approve this PR from the Android GitHub app.

@affeldt-aist
Copy link
Member Author

Looks good. The conflict with finset in the naming for direct products is fearsome, but I believe that that is already discussed when setM was introduced.

Let us ping @CohenCyril on that.

@affeldt-aist
Copy link
Member Author

@Tragicus and @proux01 were not upset by the use of setX, let's merge! ;-)

@affeldt-aist affeldt-aist merged commit d5b959a into math-comp:master Jul 31, 2024
22 checks passed
@CohenCyril
Copy link
Member

CohenCyril commented Jul 31, 2024

As @t6s noted, setX was already taken here
https://github.com/math-comp/math-comp/blob/b174dacb24a4b5d44e71e3fe18d7a5ccd25a8b85/mathcomp/ssreflect/finset.v#L54-L55
and that setM being introduced despite the pre-established convention is probably young-me's fault 🤷

In order to avoid clashes when we re-unify, why not use Y as in "sYmmetric difference", since setY is unbound for now?

@t6s
Copy link
Member

t6s commented Jul 31, 2024

As @t6s noted, setX was already taken here
https://github.com/math-comp/math-comp/blob/b174dacb24a4b5d44e71e3fe18d7a5ccd25a8b85/mathcomp/ssreflect/finset.v#L54-L55
and that setM being introduced despite the pre-established convention is probably young-me's fault 🤷

In order to avoid clashes when we re-unify, why not use Y as in "sYmmetric difference", since setY is unbound for now?

setY seems to be nice; I do not come up with any common set operation beginning with "y" :)

What about doing a renaming PR? @affeldt-aist

@affeldt-aist
Copy link
Member Author

so a new PR that turns setX into setY and renames setM into setX, let's do that
(by the way, using setY was also proposed by @proux01 and/or @Tragicus )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement ✨ This issue/PR is about adding new features enhancing the library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants