Added some MultiSet functionality #13
Conversation
| * Determine if another {@link MultiSet} is a subset of this {@link MultiSet}. A {@link MultiSet} <code>x</code> is | ||
| * a subset of <code>y</code> if for every <code>a^i</code> in <code>x</code> there exists <code>a^j</code> in | ||
| * <code>y</code> where ^ denotes the number of occurrences in the multiset. |
There was a problem hiding this comment.
Futile attempt to use a notation for occurrences.
There was a problem hiding this comment.
The notation is a challenge. I went with (a * k) in the toString. It's annoying. I don't think I like the xor/exponentiation notation but it won't prevent a merge, so don't sweat it.
There was a problem hiding this comment.
I shamelessly stole it from wikipedia and a couple papers.
7h3kk1d
force-pushed
the
multiset-additions
branch
from
35b2238
to
80f8206
Compare
| default MultiSet<A> merge(Semigroup<Natural> semigroup, MultiSet<A> other) { | ||
| return foldLeft((acc, a) -> semigroup | ||
| .apply(get(a), other.get(a)).match(constantly(acc), result -> acc.add(a, result)), | ||
| this.difference(this), // Empty but the correct impl. |
There was a problem hiding this comment.
Why do you need to start from empty? Why not just fold other, doing a get on this for the other a, applying the values via the semigroup? What am I missing here?
There was a problem hiding this comment.
You need to go over all the keys in both to make sure you still call the semigroup with zero. I could still use this but I'd have to set(via removeall + add) the value with result instead of just add.
There was a problem hiding this comment.
return foldLeft(curried(ms -> into((a, k_) -> {
Natural k = ms.get(a);
return semigroup.apply(k, k_)
.match(__ -> ms.removeAll(a),
nzk -> eq(nzk, k)
? ms
: lt(nzk, k)
? k.minus(nzk).flatMap(CoProduct2::projectB)
.match(constantly(ms), diff -> ms.add(a, diff))
: nzk.minus(k).flatMap(CoProduct2::projectB)
.match(constantly(ms), diff -> ms.remove(a, diff)));
})),
this,
other);
It's painful, but I don't like the idea of the starting difference, that seems like a hack.
I'm currently working on making MultiSet subtype Map at which point you'll get put(a, NonZero k) which will make this implementation trivial, so I won't sweat this too much right now.
There was a problem hiding this comment.
I'll add this in. Just for my own clarity, is it significantly cheaper to put by adding/removing vs removeAll + add.
7h3kk1d
force-pushed
the
multiset-additions
branch
from
80f8206
to
70fb2f4
Compare
Default Methods: - MultiSet#contains plus overload for amount - MultiSet#union - MultiSet#difference - MultiSet#inclusion - MultiSet#intersection - MultiSet#symmetricDifference - MultiSet#merge - MultiSet.EquivalenceRelations#sameElements Abstract Methods: - MultiSet#unique
7h3kk1d
force-pushed
the
multiset-additions
branch
from
70fb2f4
to
6504690
Compare
| * @see MultiSet#merge | ||
| */ | ||
| default MultiSet<A> union(MultiSet<A> other) { | ||
| return merge(max(), other); |
| * @see MultiSet#merge | ||
| */ | ||
| default MultiSet<A> intersection(MultiSet<A> other) { | ||
| return merge(min(), other); |
| * @see MultiSet#merge | ||
| */ | ||
| default MultiSet<A> symmetricDifference(MultiSet<A> other) { | ||
| return difference(other).union(other.difference(this)); |
There was a problem hiding this comment.
This stands out to me as another case where the union definition for MultiSet being the supremum makes sense, and the symmetry with Set is intuitive. Nice.
There was a problem hiding this comment.
I'm going to finish my spike of making MultiSet subtype Map: if it ends up happening, we'll want to remove unique from MultiSet as it would just be keys and make my proposed implementation of merge much simpler. If it doesn't end up panning out, I'll merge this PR in as-is.
Thanks for the wonderful work.
- Fix Javadoc typos - Stop using empty hack
- Fix MultiSet#merge - Rename MultiSet.EquivalenceRelations.sameElements
Default Methods:
Abstract Methods:
I feel pretty gross about some of the current Javadocs but I thought it would be easier to discuss over PR.