Skip to content

feat: Implement Eq and Hash for Property#94

Draft
RobertJacobsonCDC wants to merge 2 commits intomainfrom
RobertJacobsonCDC_eq_hash
Draft

feat: Implement Eq and Hash for Property#94
RobertJacobsonCDC wants to merge 2 commits intomainfrom
RobertJacobsonCDC_eq_hash

Conversation

@RobertJacobsonCDC
Copy link
Copy Markdown
Collaborator

This is an experimental branch that is compatible with this ixa branch. It changes all instances of f64 to use OrderedFloat<f64>.

This PR only exists for visibility. We won't merge this.


It turns out that using OrderedFloat<f64> is really obnoxious. Even though OrderedFloat<f64> implements all the usual operations, you still can't mix f64 and OrderedFloat<f64> for comparisons, for example. OrderedFloat<f64> implements Deref<f64> / DerefMut<f64>, but that's not a silver bullet. You end up either calling into to upcast your f64 values to OrderedFloat<f64>, or you downcast your OrderedFloat<f64> values to f64. And both ways of downcasting are obnoxious in mathematical code! You have your choice of a deref operator, an *, which looks like multiplication, or using the tuple field accessor syntax, ordered_float.0, which just as gross when you use it in a mathematical formula. You could try to only ever use OrderedFloat<f64>, avoiding naked f64 entirely. But you don't always have that option. Both the standard library and ixa uses f64 for various things like time for example. The ordered-float crate does have a feature flag for rand support, though.

Local Benchmark Results

main (33ab8db) vs RobertJacobsonCDC_eq_hash

Benchmark Base Current Change
init/population/10k 3.4157 ms 3.1341 ms -8.2% faster ✓
init/population/100k 44.433 ms 41.496 ms -6.6% faster ✓
execute/population/10k 81.324 ms 78.589 ms -3.4% faster ✓
execute/population/100k 1.2189 s 1.1451 s -6.1% faster ✓

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants