search

JuliaWaveScattering / MultipleScattering.jl

A Julia library for simulating, processing, and plotting multiple scattering of waves.
Other
45 stars 12 forks source link

Remove parameter T from most all parametric types #33

Open arturgower opened 2 years ago

arturgower commented 2 years ago

The type T, used to indicate the type of numbers used, i.e. Float64, FLoat32, etc.. appear in many parametric types:

Shape{T<:AbstractFloat,Dim}
Particle{T<:AbstractFloat,Dim,P<:PhysicalMedium,S<:Shape}

and many more... Keeping track of T is pointless, as we do not dispatch based on this in any circumstance. It only currently helps enforce that a set of types all use the same T. Enforcing this has almost no purpose, and is at times annoying.

This T should be removed from almost all parametric types.

jondea commented 2 years ago

It's necessary in the definitions of the concrete types, but you're right we should be able to get rid of it in the abstract types. Can't remember why I put them in tbh, I think I'd just learnt about parametric types and just put then everywhere 😆

arturgower commented 2 years ago

@jondea good to see you here! Yes, we both just added T everywhere when learning this stuff back in our salad days!

Even in concrete types I don't see the need to use T as a parameter. For example, the shape Sphere is currently defined as

struct Sphere{T,Dim} <: Shape{T,Dim}
    origin::SVector{Dim,T}
    radius::T
end

but I think we should instead use

struct Sphere{Dim} <: Shape{Dim}
    origin::AbstractVector{<:AbstractFloat}
    radius::AbstractFloat
end

It is important to keep track of Dim for dispatch. But the specific types of radius and origin can be left to the user. If they want efficiency, they can define types which will work better for their case. The way we currently have it, leads to more code being written in development, with no clear advantage.

jondea commented 2 years ago

If you use abstract types to define members of a struct then the types will be unknown at compile time. So the members are essentially pointers and a type (also known as boxed). This means that the probably won't be stored contiguously and functions on them will have to dynamically dispatch. If used on all low level types, you will probably see a 2-50x slowdown.

I think there's a lot of lower hanging fruit, where types can be removed or hidden. I'll have a stab at this and make a PR.

arturgower commented 2 years ago

Ah yes I remember now, coming back to me like a bad dream. Yes please do have a try at this, you are better qualified than me!

jondea commented 2 years ago

The PR I've just made should do the bulk of this, although there's a few more things that could be done. For example