Closed jonathanknowles closed 1 year ago
I tried a quick search for the origins of that requirement and I failed. It was present in the very first Git commit of the repo. The algebraic literature search didn't uncover anything either. It was already present in the original ICFP paper, though. You've already presented a convincing-enough argument as far as Haskell goes, but I'd like to see an example of a monoid that doesn't satisfy this law, just to see what we might be giving up. I don't have the time right now, but I should be able to think this through in a few days.
Ok I found some time to think. One pretty clear class of counter-examples would be any non-distributive lattice, where we define <>
= ∨
and gcd
= ∧
. The law we're discussing is exactly the definition of a distributive lattice. The Wikipedia page on lattices has some simple examples of non-distributive lattices.
A monoid-subclasses
design principle is to follow the math as closely as possible, and make the classes as general as possible. Changing the laws the way you suggest would go against this principle, because one could not for example declare an instance BoundedJoinSemiLattice a => GCDMonoid a
. What I'd suggest instead is to add a new class GCDMonoid a => DistributiveGCDMonoid a
just for the law. It's not like the library has shied from the proliferation of law-only classes before.
Hi @blamario
Many thanks for coming up with this counterexample relating to non-distributive lattices.
Regarding the proposal to add a DistributiveGCDMonoid
subclass: I've been giving this proposal some thought. I do have some initial ideas about this that I'd love to share with you and get your feedback on, though I'd need a bit of time to think them over more systematically.
Would it be okay if I get back to you in a few days? I'd probably need to wait until at least the weekend before I can give these ideas the attention they deserve. Hope that's okay!
All the best Jonathan
Of course, take your time. Ask if you have any questions, though I can't promise a prompt reply.
Hi Mario
After some thought, I agree with you that a DistributiveGCDMonoid
subclass seems very sensible. It would potentially allow us to remove the following conditional law:
If a
GCDMonoid
happens to also beCancellative
, it should additionally satisfy the following laws.
Having said that, this raises a couple of interesting questions:
LCMMonoid
? This currently requires distributivity, but under the DistributiveGCDMonoid
proposal, its superclass GCDMonoid
would not itself be distributive, as only DistributiveGCDMonoid
would have that property.Additionally, I noticed another inconsistency that would be really nice to solve:
LCMMonoid
states that lcm
must satisfy properties of idempotence, identity, associativity, and commutativity. However, the documentation for GCDMonoid.gcd
, LeftGCDMonoid.commonPrefix
, and RightGCDMonoid.commonSuffix
does not mention these properties. (Though I believe they do hold for all instances provided by the library.)With these questions in mind, what would you say to the following proposal:
First create the following classes:
-- | Class of left GCD monoids with left-distributivity.
--
-- commonPrefix (a <> b) (a <> c) == a <> commonPrefix b c
--
class LeftGCDMonoid m => LeftDistributiveGCDMonoid m
-- | Class of right GCD monoids with right-distributivity.
--
-- commonSuffix (a <> c) (b <> c) == commonSuffix a b <> c
--
class RightGCDMonoid m => RightDistributiveGCDMonoid m
DistributiveGCDMonoid
class, similar to your original suggestion, but with the above classes as superclasses:
-- | Class of GCD monoids with distributivity.
--
-- gcd (a <> b) (a <> c) == a <> gcd b c
-- gcd (a <> c) (b <> c) == gcd a b <> c
--
class (LeftDistributiveGCDMonoid m, RightDistributiveGCDMonoid m, GCDMonoid m)
=> DistributiveGCDMonoid m
DistributiveLCMMonoid
class:
-- | Class of LCM monoids with distributivity.
--
-- (laws taken from existing distributivity laws for LCMMonoid)
--
class (DistributiveGCDMonoid m, LCMMonoid m) => DistributiveLCMMonoid m
GCDMonoid
.LCMMonoid
.LCMMonoid.lcm
, also state these properties in the documentation for:
GCDMonoid.gcd
LeftGCDMonoid.commonPrefix
RightGCDMonoid.commonSuffix
I've created a prototype of this proposal in a fork of the repository here:
For this prototype:
Data.Monoid.GCD.Distributive
Data.Monoid.LCM.Distributive
monoid-subclasses
test suite. However, I have tested all the instances in a prototype branch of quickcheck-monoid-subclasses
.Would be very interested to hear your thoughts on the above proposal.
If you're happy with the general direction, but want to change anything, I'd be happy to revise it.
All the best! Jonathan
Here's a class dependency graph for the above proposal: (source)
The PR looks great so far, it just needs tests. I probably wouldn't have introduced new Distributive
modules for the new classes, it can be a nuisance to import multiple modules. I don't feel strongly about it though.
Hi @blamario
As promised, I've updated the branch and created a PR here: https://github.com/blamario/monoid-subclasses/pull/48.
Since this PR changes quite a few things, I've structured it as a series of commits. I hope that way it will be easier to review. (Each commit compiles and passes all tests.)
Regarding:
I probably wouldn't have introduced new
Distributive
modules for the new classes, it can be a nuisance to import multiple modules.
Thanks for mentioning this. I've changed the branch so that it adds new classes to the existing Data.Monoid.{GCD,LCM}
modules instead. It required a bit of adjustment to the top-level module documentation, but I'm hoping the result is better, in that it's hopefully easier for new users to understand how the Distributive
classes relate to the more general classes.
Thanks for reading through! If there's anything you'd like me to change about the PR -- let me know, would be happy to revise further. 👍🏻
Jonathan
Hi @blamario
Many thanks for reviewing and merging https://github.com/blamario/monoid-subclasses/pull/48!
I'm wondering, would it be convenient to make a new hackage release with the new Distributive
classes?
Apologies for asking again so soon after the last time -- I know you already made a few releases recently. (If there's anything I can do to assist, am happy to help.)
All the best
Jonathan
No problem, just released.
No problem, just released.
Thanks!
Hi @blamario
While reading through the documentation for
GCDMonoid
, I noticed this statement about distributivity laws:This got me thinking: must a GCD monoid be cancellative in order to satisfy the above laws?
Among the types for which instances of
GCDMonoid
are provided, only the following types are notCancellative
:IntSet
Set a
Product Natural
But despite non-cancellativity, these types do seem to satisfy the distributivity laws:
Justification
Set types
We can consider the
IntSet
andSet a
types together, as theirGCDMonoid
andSemigroup
instances are defined equivalently:Of course, these types are certainly not
Cancellative
:However, we can show that they satisfy the distributivity laws.
If we substitute the above instance method definitions into the distributivity laws, we obtain:
Rewriting with traditional set notation, we obtain:
These are equivalent to the standard distributivity law(s) for sets, which can be proven in a number of different ways.
Product Natural
This type has the following
Semigroup
andGCDMonoid
instances:This type is also not
Cancellative
:But again, we can show that
Product Natural
satisfies the GCD distributivity laws.If we substitute the instance method definitions above into the distributivity laws, we obtain:
This is equivalent to the distributivity laws for the standard gcd function on non-zero natural numbers:
But what about values of
Product 0
? Well it turns out that the laws still hold for zero values, sincePrelude.gcd
is defined such that:Which means that:
Proposal
Given that all the provided instances of
GCDMonoid
do appear to satisfy the distributivity laws (regardless of whether or not they areCancellative
), perhaps could we simply revise the documentation forGCDMonoid
to state this as a requirement, namely that all instances must satisfy the distributivity laws?Advantages:
gcd
-like operations. It seems a shame that library writers can only rely on this property for the subset ofGCDMonoid
types that are alsoCancellative
.GCDMonoid
type class more consistent withLCMMonoid
, which does require that all instances satisfy the distributivity laws.I did a search over all code in GitHub that uses
monoid-subclasses
, but couldn't find an instance (out in the wild) that would violate these laws. I also tested all the provided instances extensively against a suite of property tests with coverage checks, and could not find a counterexample.But perhaps I'm overlooking something? Perhaps there a good reason to keep this restriction?
If you agree this is a worthwhile simplification, let me know -- I'd be happy to make a PR that adjusts the tests and documentation. 👍🏻
Thanks for reading through, and best wishes!
Jonathan