implement Hash for Antichain<T> when T is Ord+Hash #454
Conversation
This allows for e.g. indexing differential Descriptions by their lower to stitch them together.
There was a problem hiding this comment.
@frankmcsherry I double-checked after office hours and it looked like we indeed didn't have this. no hurry at all! we might want something like this eventually, but not soon
|
|
||
| impl<T: Ord+std::hash::Hash> std::hash::Hash for Antichain<T> { | ||
| fn hash<H: std::hash::Hasher>(&self, state: &mut H) { | ||
| let mut temp = self.elements.iter().collect::<Vec<_>>(); |
There was a problem hiding this comment.
this temp Vec is unfortunate, should I be doing something different? maybe impl Hash for MutableAntichain (which requires that T be Ord) instead?
There was a problem hiding this comment.
This seems fine to me, and is in-line with how we do equality. It just means that Hash is not super cheap, but that isn't the end of the world imo. A different take is that we should maintain antichains in normal form (sorted, say), which is a different amount of disruptive.
There was a problem hiding this comment.
Okay then it works for me, too! It's still much faster than the alternative (repeatedly iterating through a list to call Eq). Just wanted to make sure I wasn't missing something.
|
Looks good, and thanks! |
2 participants
This allows for e.g. indexing differential Descriptions by their lower
to stitch them together.