-
Notifications
You must be signed in to change notification settings - Fork 247
Add properties of algebraic semilattices #544
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
Conversation
Thanks a lot, that's great! I've just merged #535 in, which redefines lattices in terms of semilattices so you may need to do a little tweaking of the PR... apologies! Edit: Also could you document which proofs have been moved in the CHANGELOG. Thanks! |
7b2ee95
to
cf81aff
Compare
@MatthewDaggitt: OK that should be easy to fix, but is defining lattices in terms of semilattices really a good idea? Before, idempotence of the meet and join operations was a consequence of absorption (which was proved in |
Hmm that's a good point. Compositionality is better with the new structure, but the downside is that idempotence must be proved directly... I guess that the changes should probably be reverted. Apologies, I don't have time at the moment, but I'll try and get round to it in the next couple of weeks. |
cf81aff
to
fc72882
Compare
OK, I fixed the PR so that it should hopefully pass the CI now, commented out the (now redundant) idempotence theorems in I'll leave it up to you whether to merge the PR now or later. In either case, it should be easy to adjust |
Actually, I still have to update the CHANGELOG as well before the PR can be merged. If you want, I can update the PR to include a (separate) commit reverting the changes to the lattice records to make things easier. WDYT? |
Compositionality is better with the new structure, but the downside is that idempotence must be proved directly...
One could still provide a smart constructor that takes the old `field`s.
|
Hmm it would be quite a long constructor... Passing 8(?) parameters to it isn't going to be very neat. Given the downsides, probably better to err on the side of minimising non-backwards compatible changes.
If you have the time @sstucki that would be great, thanks! |
fc72882
to
beae48a
Compare
beae48a
to
aa1f368
Compare
Here you go @MatthewDaggitt. |
@MatthewDaggitt, let me know if there's anything else you need from me to merge this PR. |
Nope, looks good thanks! Merging in |
OK, thanks! |
@sstucki I forgot to ask. Given that you've contributed modules, would you like to be added to authors list and if so under what name? |
Sure! You can add me as Sandro Stucki. Thanks and happy holidays! |
...and update dependencies.
I noticed that algebraic semilattices are now defined as a structure in the
Algebra
module, so I thought it might be nice to also add aAlgebra.Properties
module for them (similar to that for lattices).I moved some of the properties from
Algebra.Properties.Lattice
toAlgebra.Properties.Semilattice
and updated the former so as not to break dependencies.