Error in multiple dispatch

Hi, I am trying to learn multiple dispatch and I can’t figure out why the following code is not working.

module GeoLocation

using TimeZones
using DynamicQuantities
using Unitful: Unitful
using DocStringExtensions

export Location

"""
$(TYPEDEF)
"""
struct Location
    "Latitude in degrees"
    latitude::Quantity
    "Longitude in degrees"
    longitude::Quantity
    "Altitude in meters"
    altitude::Quantity
    "Timezone of the location"
    timezone::FixedTimeZone
end

# constructor accepting Float64 inputs
function Location(;
    latitude::Float64,
    longitude::Float64,
    altitude::Float64 = 0.0,
    timezone::FixedTimeZone = TimeZone("UTC"),
)
    return Location(
        Quantity(latitude, deg = 1),
        Quantity(longitude, deg = 1),
        Quantity(altitude, m = 1),
        timezone,
    )
end

# constructor accepting Quantity inputs
function Location(;
    latitude::Quantity,
    longitude::Quantity,
    altitude::Quantity = Quantity(0.0, m = 1),
    timezone::FixedTimeZone = TimeZone("UTC"),
)
    return Location(latitude, longitude, altitude, timezone)
end

end

So basically I want to have two constructors, one for quantities and one for floats.

Quantities work fine:

julia> location1 = Location(latitude = Quantity(1.0, deg = 1), longitude = Quantity(1.0, deg = 1))
Location(1.0 , 1.0 , 0.0 , tz"UTC")

Floats fail:

julia> location1 = Location(latitude = 1.0, longitude = 1.0)
ERROR: TypeError: in keyword argument latitude, expected Quantity, got a value of type Float64
Stacktrace:
 [1] top-level scope
   @ REPL[17]:1

You can only distinguish multiple methods by positional arguments, which is how multiple dispatch can work. Your 2 method definitions have the same (0) positional arguments, so the 2nd one overwrote the 1st one. If you really need keyword arguments, you can merge the 2 definitions and either branch or use a helper method* to handle the different types. The compiler will still compile the 1 method to separate specialized code for the different input types.

*The easiest pattern would be to make multiple methods with differently annotated positional arguments, but make 1 keyword argument method that just forwards to the positional call.

4 Likes

Hi,
in super short: Multiple dispatch does not work for keyword arguments, see also Using Multiple Dispatch with Keyword Arguments

What I assume is happening here is, that you second definition is not an additional one, but it redefines (hence replaces) the zero-argument constructor If you switch both definitions, the float one will work, the quantity one will not.

You could use positional arguments instead

2 Likes

To do what you want do this instead:

# constructor accepting only keyword arguments
@inline function Location(;
    latitude,
    longitude,
    altitude = zero(typeof(latitude)),
    timezone::FixedTimeZone = TimeZone("UTC")
)
    # Dispatch keywords to positional arguments
    return Location(
        latitude,
        longitude,
        altitude,
        timezone
    )
end

# constructor accepting Float64 inputs
function Location(
    latitude::Float64,
    longitude::Float64,
    altitude::Float64 = 0.0,
    timezone::FixedTimeZone = TimeZone("UTC"),
)
    return Location(
        Quantity(latitude, deg = 1),
        Quantity(longitude, deg = 1),
        Quantity(altitude, m = 1),
        timezone,
    )
end

# constructor accepting Quantity inputs
function Location(
    latitude::Quantity,
    longitude::Quantity,
    altitude::Quantity = Quantity(0.0, m = 1),
    timezone::FixedTimeZone = TimeZone("UTC"),
)
    return Location(latitude, longitude, altitude, timezone)
end
5 Likes

Thanks. This is giving a stack overflow error when I run it, so probably it’s constructing itself in a loop.

You’ll definitely need to make some edits to that code. The stackoverflow is caused by the last method definition making a method (the one where you don’t use the default values) that overwrote the default (inner) constructor that puts the struct together. There’s recursion, but there’s no construction/instantiation at all. The fix would be to make sure none of your outer constructors have argument annotations that match the default constructors (at least one would use the struct field annotations).

  1. You could make a custom inner constructor, which is called by all other constructors. Move that method signature into the struct definition and call new instead.
  2. You know you overwrote an inner Location(latitude::Quantity, longitude::Quantity, altitude::Quantity, timezone::FixedTimeZone method, so the outer definition should avoid it by omitting the timezone argument, just inline TimeZone("UTC").

The other immediately noticeable mistake is zero(typeof(latitude)) in the keyword argument method; if you use Quantity inputs, then zero won’t get you to the Quantity(0.0, m=1) you want.

1 Like

I don’t understand what you mean by inline here?

I got this now:

struct Location
    "Latitude in degrees"
    latitude::Quantity
    "Longitude in degrees"
    longitude::Quantity
    "Altitude in meters"
    altitude::Quantity
    "Timezone of the location"
    timezone::FixedTimeZone

    function Location(
        latitude::Quantity,
        longitude::Quantity,
        altitude::Quantity,
        timezone::FixedTimeZone,
    )
        return new(latitude, longitude, altitude, timezone)
    end
end

# constructor accepting only keyword arguments
@inline function Location(;
    latitude,
    longitude,
    altitude = 0.0,
    timezone::FixedTimeZone = TimeZone("UTC"),
)
    # Dispatch keywords to positional arguments
    return LocationOuter(latitude, longitude, altitude, timezone)
end

# constructor accepting Float64 inputs
function LocationOuter(
    latitude::Float64,
    longitude::Float64,
    altitude::Float64 = 0.0,
    timezone::FixedTimeZone = TimeZone("UTC"),
)
    return Location(
        Quantity(latitude, deg = 1),
        Quantity(longitude, deg = 1),
        Quantity(altitude, m = 1),
        timezone,
    )
end

# constructor accepting Quantity inputs
function LocationOuter(
    latitude::Quantity,
    longitude::Quantity,
    altitude::Quantity = Quantity(0.0, m = 1),
    timezone::FixedTimeZone = TimeZone("UTC"),
)
    return Location(latitude, longitude, altitude, timezone)
end

But this code has two problems:

  1. I can’t set an altitude default on: @inline function Location(; latitude, longitude, altitude = 0.0, timezone::FixedTimeZone = TimeZone("UTC"),) because that breaks the dispatch
  2. The readability of this code is terrible

There is no need to define the inner one. Whether code is terrible or not is of course also a bit a matter of taste. I tried to simplify the code a bit in the following. All new constructors in the end fall back to call the “original” constructor that just accepts the 4 fields.

For the default – that is a bit tricky, since in the keywords you have to decide which default you want to provide and the “other case” has to treat that accordingly.
My solution is to set a default alt of 0.0 as a float and have a second constructor that just “converts” a float alt to a Quantity (when the rest is quantities.

Would you consider the following more readable?


using TimeZones
using DynamicQuantities
using Unitful: Unitful

struct Location
    "Latitude in degrees"
    latitude::Quantity
    "Longitude in degrees"
    longitude::Quantity
    "Altitude in meters"
    altitude::Quantity
    "Timezone of the location"
    timezone::FixedTimeZone
end

# constructor accepting keywords
function Location(;
    latitude,
    longitude,
    altitude = 0.0,
    timezone = TimeZone("UTC"),
)
    return Location(latitude, longitude, altitude, timezone)
end

function Location(latitude::F, longitude::F, altitude::F, timezone::FixedTimeZone) where {F<:Real}
    return Location(Quantity(latitude, deg = 1), Quantity(longitude, deg = 1), Quantity(altitude, m = 1), timezone)
end
# Handle default for Quantities
function Location(latitude::Q, longitude::Q, altitude::F, timezone::FixedTimeZone) where {F<:Real, Q<:Quantity}
    return Location(latitude, longitude, Quantity(altitude, m = 1), timezone)
end

When all 3 Quantities are already Quantities, the keyword constructor just calls the “original/default” one already, no need to define that then

Example use cases:

# Timezone is always default in the following ones
# Constructor 1 Quantities
Location(; latitude=Quantity(1.0), longitude=Quantity(2.0), altitude=Quantity(7.0))
# Constructor 1 Quantities, default altitude
Location(; latitude=Quantity(1.0), longitude=Quantity(2.0))
# Constructor 3 Float
Location(; latitude=1.0, longitude=2.0, altitude=7.0)
# Constructor 4 Float, default altitude
Location(; latitude=1.0, longitude=2.0)

# For completeness, new time zone, default altitude
Location(; latitude=1.0, longitude=2.0, timezone=TimeZone("UTC-2"))
3 Likes

Yes this works great and is readable, thank you very much. It’s a nice educational example as well.

1 Like

Just to clarify, the (1) and (2) I wrote are not steps in 1 option, but 2 distinct options, don’t do those at the same time. By (2), I mean making sure that the last definition only makes outer constructors with 2 or 3 arguments:

function Location(
    latitude::Quantity,
    longitude::Quantity,
    altitude::Quantity = Quantity(0.0, m = 1)
)
    return Location(latitude, longitude, altitude, TimeZone("UTC")) # inlined value
end

so you don’t overwrite the default 4-argument constructor with 3 Quantity and a TimeZone that was defined by the struct definition. The same awareness of methods to be called and defined is used here to clean up the methods and lessen the use of default arguments:

Absolutely use kellertuer’s version. If you decide to only support Float64, editing F<:Real to F<:Float64 would work, the outer constructors still won’t collide with the struct definition’s constructors either.

2 Likes

Very good addition! Yes, make sure to not “accidentially” define the existing (inner, default) constructor of the struct with an own outer one, like all the ones we defined.

And yeah, I was maybe not so detailed about my definition – when I tried to make the code a bit more compact, allowing more than just Float64 was easier to write even, so I went for that.

1 Like