EnergySystemsModellingLab / EnergySystemsModellingLab/MUSE2

Implement `Ord` and `Eq` for unit types

Open
#717 2 comments 0 reactions 0 assignees View on GitHub
question
Dominant language
Rust
Stars
8
Forks
5
Avg merge
1d 2m
Merged PRs (30d)
37

Description

> Thanks for explaining. It's probably fine as it is then, but if `ordered_float` is easy to implement then it might be nice.

I'll take another look tomorrow to see how faffy it is. Maybe it should be a separate PR.

> Worth thinking about nans though. I don't imagine we'll ever want unit types to be nan, but there are certainly times where these could crop up and we'll need to deal with them. I wonder if [`NotNan`](https://docs.rs/ordered-float/latest/ordered_float/struct.NotNan.html) could be something to consider. This will panic if nans are ever encountered, which will force us to explicitly deal with these scenarios (e.g. adding an epsilon to prevent a 0/0 division)

Yeah, I don't think so either. I think it would be a bit more Rusty to have an `Option` rather than an `Activity` that might be NaN. I've grepped the code base and it looks like the only NaN unit types that exist are in tests, so it *might* be safe to swap over.

My only worry is that there could be performance implications to the extra checks that would be implicitly needed for all kinds of floating point operations, but maybe that's not worth worrying about.

The other thing is that we would need to create unit types with a constructor, because writing e.g. `Activity(1.0)` wouldn't work anymore.

_Originally posted by @alexdewar in https://github.com/EnergySystemsModellingLab/MUSE_2.0/issues/707#issuecomment-3103737555_

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.