-
Notifications
You must be signed in to change notification settings - Fork 484
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Migrate from Float
/Double
to T: FloatingPoint
#147
Comments
A few questions:
|
It wouldn't, unfortunately. 😕 I would expect the API to I agree though, that this might not be the desired behavior for a default API. I wish SwiftPM allowed for specifying settings on dependencies (e.g. Alas SwiftPM only allows for settings on direct targets. |
I think this would be a step backwards in terms of API usability. Taking the current approach as a baseline, the proposed change would:
By comparison, the proposed alternative to use code generation would solve 1) without introducing 2) or 3). So currently, I'm leaning towards this alternative approach. I also think it paints us into a corner when it comes to supporting additional floating point types should they be introduced. Imagine if a future version of Swift and Accelerate adds a If Surge 3.0 makes functions generic — supporting only Or actually, here's the worst-case scenario: Let's say that a In another thread you mentioned that:
I was hoping to draw that out, as I think that's the information I'm missing to understand the motivation for this change. Could you tell me more about what you've seen in this respect? Do you have examples of how people are using |
Well, that didn't take long! apple/swift-numerics#8 |
Code generation alone unfortunately does not solve the issue that without an API with signature My goal with adding (or merging into)
It actually doesn't take an addition of
100% agree.
Again, I 100% agree. My best-case solution would be to have a way to generate a generic wrapper for a concrete (i.e.
And it happens I might have found a proof-of-concept solution for exactly that, actually. I'll keep you posted. Don't want to promise too much, too early.
Well, for me personally as someone who does their other 50% share of coding mostly in Rust I tend to aim for my code to be maximally generic in Swift, too. It's a habit, I guess. But judging from the existence of projects such as … https://github.com/cloutiertyler/Jolt (abandoned) … it seems like I'm not alone in this.
Heh, nice! It won't really change that much for Surge though, will it? |
With Swift already supporting As such initial thought was to provide a specialized protocol // Consider this protocol "closed", i.e. NO types must conform to it other than the ones listed below.
public protocol SurgeFloatingPoint: FloatingPoint {}
extension Float: SurgeFloatingPoint {}
extension Double: SurgeFloatingPoint {} While this would provide a safe API as long as the user plays by the rules it is far from optimal, in respect to type-safety. Not all is lost though! While Swift doesn't have closed protocols @anandabits found a brilliant way to make protocols effectively closed: // Adapted from: https://gist.github.com/anandabits/bd73521e0c5c06371f4a268ab8c482c9
// This somewhat obscure code emulates `closed protocol` semantics for `SurgeFloatingPoint`:
/* closed */ public protocol SurgeFloatingPoint: BinaryFloatingPoint {
associatedtype _Validator: _SurgeFloatingPointValidator where _Validator.Validated == Self
}
public class _SurgeFloatingPointValidatorBase {}
public protocol _SurgeFloatingPointValidator where Self: _SurgeFloatingPointValidatorBase {
associatedtype Validated
}
extension Float: SurgeFloatingPoint {
public final class _Validator: _SurgeFloatingPointValidatorBase, _SurgeFloatingPointValidator {
public typealias Validated = Float
}
}
extension Double: SurgeFloatingPoint {
public final class _Validator: _SurgeFloatingPointValidatorBase, _SurgeFloatingPointValidator {
public typealias Validated = Double
}
} With this in place Surge could provide a generic API for |
tl;dr
Surge currently provides separate implementations for each function for
Float
andDouble
, respectively. This makes Surge basically incompatible with Swift'sT: FloatingPoint
generics. By introducing a little bit of internal runtime dynamism we aim to migrate existing function pairs to their generic equivalent forT: FloatingPoint
.What?
With the recent refactors we have managed to reduce the implementations of each computation into a function set consisting of a single
internal
core-implementation, acting as a single source of truth, and a bunch of thinpublic
convenience wrapper functions.Scalar-Division (
[Scalar] / Scalar
) is implemented like this:… with an almost identical copy existing for each of these functions for
Double
, instead ofFloat
.Why?
While the project's current state is quite an improvement over its previous state it has a couple of remaining deficits:
Float
andDouble
.T: FloatingPoint
overFloat
/Double
.So this got me thinking: What if we migrated Surge from using
Float
/Double
to an API withT: FloatingPoint
and then internally make use of some dynamic language features to roll our own polymorphism over the closed set ofFloat
andDouble
with afatalError(…)
on type-mismatch?Aforementioned dynamism would add a certain amount of runtime overhead to Surge. It is important to note however that we would be adding a constant overhead (
O(1)
vs.O(N)
), as a single call ofSurge.divInPlace(_:_:)
over a pair of10_000
-element arrays only adds a single branch per execution, not10_000
branches in a loop, as would be the case for a naïve non-parallel looping implementation.How?
So how would this look like? What would we need to change?
public
wrapper functions forFloat
/Double
with a single equivalent function that is generic overT: FloatingPoint
, instead.internal
…InPlace(…)
core-implementation functions forFloat
/Double
into a single equivalent function that is generic overT: FloatingPoint
on the outside and then performs aswitch
onT.self
on the inside, instead.func withMemoryRebound(to:_:)
toUnsafeMemory<T>
andUnsafeMutableMemory<T>
, so that we can efficiently cast fromUnsafeMemory<T: FloatingPoint>
toUnsafeMemory<Double>
, without having to copy/cast any individual values.func withUnsafeMemory(as:…)
convenience functions for retrieving type-cast variants ofUnsafeMemory<T>
from instances ofUnsafeMemoryAccessible
/UnsafeMutableMemoryAccessible
.func …InPlace(…)
implementations into something like this:So far I have not been able to measure any noticeable performance regressions introduced by this change.
There also should be very little breakage from the changes, as
T: FloatingPoint
is for the most part a strict superset of eitherFloat
orDouble
.(I already have a proof-of-concept for this on a local branch and will push it as a PR at some point.)
The text was updated successfully, but these errors were encountered: