morganthomas / purescript-group

Algebraic groups for PureScript.
Apache License 2.0
6 stars 6 forks source link

Add Free #5

Closed hrb90 closed 6 years ago

hrb90 commented 6 years ago

Adds an instance for the Free group on a given set of generators.

Does require a couple of new dependencies (lists and foldable-traversable)

Not sure if there are any other instances that Free should have.

hrb90 commented 6 years ago

Some Slack conversation exposed another construction we might consider for implementation of free groups:

data FreeGrp a = GEmpty | GConsPlus a (FreeGrp a) | GConsMinus a (FreeGrp a)

I'm not sure which would be better from an API design perspective and want to solicit feedback :)

morganthomas commented 6 years ago

Good idea! This looks right to me from a math perspective.

morganthomas commented 6 years ago

In my opinion the current approach is nicer than the one suggested on Slack. But is there a reason to prefer the Slack suggestion?

hrb90 commented 6 years ago

Yes, I like the Signed approach better as well (and the Haskell library that exposes free groups does the same thing, for whatever that's worth.)

The upside of the GConsPlus/GConsMinus construction was that it might make the parallels to other free constructions even clearer, in that it "lifts the laws of the algebraic structure to data constructors" or something along those lines. I'm sorry if that's not too clear; I'm probably not a great advocate for something I don't really want to reimplement.

By the way, I went ahead and published this as its own library on Pursuit under the name purescript-free-group. (What can I say, I was excited to publish a library.) I still think it probably fits better as a part of purescript-group, and I'll deprecate the package if this PR gets accepted, but I would like to port over the changes I've made to this branch.

morganthomas commented 6 years ago

@hrb90 Let me know when(/if) you have something you feel is ready to be merged. Thank you! This is a very nice addition to the package imo.

hrb90 commented 6 years ago

Updated the code, think it's ready to merge :smile:

morganthomas commented 6 years ago

Merged, changed names as described, bumped version, published to Pursuit!