Skip to content

Feature/generic floats - #329

Merged
elnaske merged 6 commits into
bglid:mainfrom
elnaske:feature/generic-floats
Sep 29, 2026
Merged

elnaske merged 6 commits into
bglid:mainfrom
elnaske:feature/generic-floats

Conversation

@elnaske

@elnaske elnaske commented Sep 28, 2026 •

Copy link
Copy Markdown
Collaborator

Description

Replaces the explicit f64s with generics that implement the Float trait from the num_traits crate (or, more specifically, a supertrait that requires num_traits::Float plus some other traits needed for common ops), to allow the code to be used with f32s as well.

This has several advantages: Single-precision floats take up less memory, which makes them more suited to resource-constrained devices. They may also lead to increased performance, since twice as many of them can fit in the same cache line, which should, in theory, reduce the number of cache misses.

One small downside is that arithmetic operations can no longer automatically dereference floats.
For instance, previously we had:

// iter() returns &f64, which is automatically deref'd in `x * x`
noise_ref.iter().map(|x| x * x).sum();

We now have to manually dereference:

noise_ref.iter().map(|x| (*x) * (*x)).sum();

Or use a copied iterator:

noise_ref.iter().copied().map(|x| x * x).sum();

I prefer the second one because it's easier to read, and floats are so small that the copies are free.

@elnaske

elnaske commented Sep 28, 2026 •

Copy link
Copy Markdown
Collaborator Author

@bglid This is mostly done, so you can have a look already. I'll probably just add some type aliases to make syntax less cumbersome, e.g.:

type FilterBase32<A> = FilterBase<A, f32>;
type FilterBase64<A> = FilterBase<A, f64>;

(also have to figure out if I need to make sure that the algorithm uses the same float type as the filter base)

Also want to hear your preference for the ordering of the Algorithm and Float traits in the definition of FilterBase.
I.e. it would be between
FilterBase<A: Algorithm<F>, F: Float> (e.g. FilterBase<Lms<f64>, f64>) and
FilterBase<F: Float, A: Algorithm<F>> (e.g. FilterBase<f64, Lms<f64>>).
Right now I have it as the prior, but I'm thinking about changing it to the latter.

@elnaske

elnaske commented Sep 28, 2026

Copy link
Copy Markdown
Collaborator Author

Another thing I forgot: num_traits::Floatgives you all the ops you could ever want for a float, but since we'll probably only need a few of them, we could later on define our own Float trait. That would involve some boilerplate (although that could be alleviated with macros), but we could get rid of a dependency that we only need for this one trait

@elnaske
elnaske requested a review from bglid September 28, 2026 20:23
@bglid

bglid commented Sep 29, 2026

Copy link
Copy Markdown
Owner

Description

Replaces the explicit f64s with generics that implement the Float trait from the num_traits crate (or, more specifically, a supertrait that requires num_traits::Float plus some other traits needed for common ops), to allow the code to be used with f32s as well.

This has several advantages: Single-precision floats take up less memory, which makes them more suited to resource-constrained devices. They may also lead to increased performance, since twice as many of them can fit in the same cache line, which should, in theory, reduce the number of cache misses.

One small downside is that arithmetic operations can no longer automatically dereference floats. For instance, previously we had:

// iter() returns &f64, which is automatically deref'd in `x * x`
noise_ref.iter().map(|x| x * x).sum();

We now have to manually dereference:

noise_ref.iter().map(|x| (*x) * (*x)).sum();

Or use a copied iterator:

noise_ref.iter().copied().map(|x| x * x).sum();

I prefer the second one because it's easier to read, and floats are so small that the copies are free.

Yeah I agree, I prefer the second one too. Much more readable. Worthy tradeoff too

@bglid

bglid commented Sep 29, 2026

Copy link
Copy Markdown
Owner

@bglid This is mostly done, so you can have a look already. I'll probably just add some type aliases to make syntax less cumbersome, e.g.:

type FilterBase32<A> = FilterBase<A, f32>;
type FilterBase64<A> = FilterBase<A, f64>;

(also have to figure out if I need to make sure that the algorithm uses the same float type as the filter base)

Also want to hear your preference for the ordering of the Algorithm and Float traits in the definition of FilterBase. I.e. it would be between FilterBase<A: Algorithm<F>, F: Float> (e.g. FilterBase<Lms<f64>, f64>) and FilterBase<F: Float, A: Algorithm<F>> (e.g. FilterBase<f64, Lms<f64>>). Right now I have it as the prior, but I'm thinking about changing it to the latter.

Hmmm without thinking too hard on this, my gut reaction is Algorithm then Float. But, I think the second one does look maybe a bit more legible because it has the Algorithm serving as a delineator between the two floats.

Another thing I forgot: num_traits::Floatgives you all the ops you could ever want for a float, but since we'll probably only need a few of them, we could later on define our own Float trait. That would involve some boilerplate (although that could be alleviated with macros), but we could get rid of a dependency that we only need for this one trait

Oh yeah I think this is worth doing then. Plus could be fun to do too

Comment thread examples/custom_algorithm.rs Outdated
Comment thread src/algorithms/lms.rs
Comment thread src/error.rs Outdated
@bglid

bglid commented Sep 29, 2026

Copy link
Copy Markdown
Owner

Gave the approval cause I'll be away from my computer for most of the day. I trust that once you finish this it'll be good to merge

@elnaske

elnaske commented Sep 29, 2026

Copy link
Copy Markdown
Collaborator Author

We might not need the aliases after all. This works just fine:

let lms = Lms::new(1.0_f32).unwrap();
let filter = FilterBase::new(lms, 1024).unwrap();

The compiler can figure out that filter is FilterBase<Lms<f32>, f32> without further annotation.

Also I checked and the compiler ensures that the float type for the algorithm and for the filter have to be the same.

@elnaske
elnaske marked this pull request as ready for review September 29, 2026 18:37
@elnaske
elnaske merged commit c261c39 into bglid:main Sep 29, 2026
2 checks passed
@elnaske
elnaske deleted the feature/generic-floats branch September 29, 2026 18:59
@bglid

bglid commented Sep 30, 2026

Copy link
Copy Markdown
Owner

We might not need the aliases after all. This works just fine:

let lms = Lms::new(1.0_f32).unwrap();
let filter = FilterBase::new(lms, 1024).unwrap();

The compiler can figure out that filter is FilterBase<Lms<f32>, f32> without further annotation.

Also I checked and the compiler ensures that the float type for the algorithm and for the filter have to be the same.

Oh swweet that's great

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