Method works in REPL; can't use in a function body

I’ve stared at this a long time and can’t figure out what’s wrong.

Here is a method call that works in the REPL:

julia> plus!([1,2,3,4,5], infectious, 1:5,1,1)
5-element view(::Array{Float64,4}, 2, 1, 1:5, 1) with eltype Float64:
  6.0
  7.0
  8.0
  9.0
 10.0

(Note: the view is just for returning the result in the REPL.)

Here is the same method used in the body of a function the same .jl file:

function spread(locale)
    # start with the number of infectious people      # now we ignore what their condition and age are is: TODO fix
    spreaders = Int(sum(grab(infectious, 1:5,1:11, locale)))

    # how many people are touched 
    touched = how_many_touched(spreaders)

    # by age group
    byage = split_by_age(touched)

    # move the people from unexposed:agegrp to infectious:agegrp
    plus!(byage, infectious, agegrps, 1, locale)

end

Here is the resulting error message:

julia> spread(1)
ERROR: MethodError: no method matching plus!(::Array{Int64,1}, ::Int64, ::Array{Int64,1}, ::Int64, ::Int64)
Closest candidates are:
  plus!(::Union{Real, Array}, ::Union{UnitRange{Int64}, Int64}, ::Union{UnitRange{Int64}, Int64}, ::Union{UnitRange{Int64}, Int64}, ::Union{UnitRange{Int64}, Int64}; dat) at /Users/lewis/Dropbox/Online Coursework/Covid/prototype.jl:290
Stacktrace:
 [1] spread(::Int64) at /Users/~/Dropbox/Online Coursework/Covid/prototype.jl:178
 [2] top-level scope at REPL[8]:1

Here is the method definition:

function plus!(val, condition::Union{Int, UnitRange{Int}}, agegrp::Union{Int, UnitRange{Int}}, 
    lag::Union{Int, UnitRange{Int}}, locale::Union{Int, UnitRange{Int}}; dat=openmx)
    dat[condition,locale, agegrp,lag] .+= val
end

I must be missing something obvious. Any thoughts?

It is hard to debug this without an MWE.

There is probably a global variable that does not have the value you think it has. Try to write cleaner code, passing all arguments needed to a function — eg infectious for spread.

4 Likes

Your third argument is restricted to either Ints or UnitRanges, but the third argument you passed it is a vector (::Array{Int64,1}).

Also, is it a global variable? I’d recommend avoiding globals if you can.

1 Like

You appear to be going overboard on typing your input variables. Start out without any types at all in the signatures, and then add them as needed. Now you’re just making problems for yourself, and making the code hard to read.

Also, definitely add an MWE.

3 Likes

I suggest that you redefine the plus! method like this

function plus!(val, condition, agegrp, lag, locale; dat=openmx)
    dat[condition, locale, agegrp, lag] += val
end

and then call it like this:

plus!.(byage, infectious, agegrps, 1, locale)

This is cleaner, and should be faster, since your original implementation creates a temporary array on the right hand side of the assignment.

Putting the broadcasting dot on the outside, instead of inside your function is generally the way to go. The broadcasting functionality was specifically made to avoid this sort of ugly ::Union{UnitRange{Int64}, Int64} type signatures.

1 Like

Thanks.

Believe me, I started with very small functions and testing them individually. Incremental MWE.

Thanks for the hint about dotting when calling the function.

The reason for typing so much that I had to differentiate methods to work with a scalar input and another for vector or broadcasting. But, putting the dot inside I needed to .+= so two methods. By having the caller dot the function,
I don’t need to differentiate the method by range or scalar. Both problems solved.

Oops. I missed one!

There are globals and they are all constants: not just in type, but also in value. They are more like mnemonic aliases as an alternative to using ints to reference semantic parts of arrays.

It is decently clean code. Globals are all constants (and values not changing, not just types). They are used as aliases for integer indices into the arrays.

1 Like

This is not apparent from the code you posted.

Again, without an MWE, it is difficult to help.

Although in this case, replacing arguments with underscores to de-emphasize them:

ERROR: MethodError: no method matching plus!(__, __, ::Array{Int64,1}, __, __)
Closest candidates are:
  plus!(__, __, ::Union{UnitRange{Int64}, Int64}, __, __; __) at /Users/lewis/Dropbox/Online Coursework/Covid/prototype.jl:290
1 Like

Why is there a performance benefit for a const declaration?

Can you explain how that reconciles with this from the manual?

The const declaration should only be used in global scope on globals. It is difficult for the compiler to optimize code involving global variables, since their values (or even their types) might change at almost any time. If a global variable
will not change*, adding a const declaration solves this performance problem.*

Note that const only affects the variable binding; the variable may be bound to a mutable object (such as an array), and that object may still be modified. Additionally when one
tries to assign a value to a variable that is declared constant the following scenarios are possible:

<there are several cases of warnings or errors…>

Typically this means that a const primitive type cannot have its value changed, raising an exception. However, if the elements of a composite type are not individually constant then the elements’ values can be changed because the compiler
still has a contract for the “constancy” of the type of the variable.

As this code is developed, there are many potential function parameters, many which are semantically constant—that is, they are parameters that will not be changed by the running code. They are inputs that set initial conditions. Once
the code stabilizes and it is obvious which are needed in various functions, I’ll likely wrap them up in struct packages and use the structs as arguments to keep the sizes of the argument lists manageable.

As declared constants whose values are read not altered, there seems little performance consequence.

I might be wrong about how I understand this….

The basic benefit of a constant declaration is that the compiler is sure that the type of that variable won’t change later. So, it can generate specialized code.

If you try to change the type of a variable declared as a constant, you get an error. You can change the value of a variable declared as a constant, but not the type.

julia> const a = 1
1

julia> a = 1.2
ERROR: invalid redefinition of constant a
Stacktrace:
 [1] top-level scope at REPL[2]:1

julia> a = 3
WARNING: redefining constant a
3

But changing the value is a bad idea.

julia> const a = 3
3

julia> foo(x) = x * a
foo (generic function with 1 method)

julia> foo(5)
15

julia> @code_typed foo(5)
CodeInfo(
1 ─ %1 = Main.a::Core.Compiler.Const(3, false)
│   %2 = Base.mul_int(x, %1)::Int64
└──      return %2
) => Int64

julia> const a = 7
WARNING: redefining constant a
7

julia> foo(5)
15

julia> foo(5.0)
35.0

julia> @code_typed foo(5)
CodeInfo(
1 ─ %1 = Main.a::Core.Compiler.Const(7, false)
│   %2 = Base.mul_int(x, %1)::Int64
└──      return %2
) => Int64

julia> @code_typed foo(5.0)
CodeInfo(
1 ─ %1 = Main.a::Core.Compiler.Const(7, false)
│   %2 = Base.sitofp(Float64, %1)::Float64
│   %3 = Base.mul_float(x, %2)::Float64
└──      return %3
) => Float64

When you redefine a function, functions using it will automatically recompile. But when you redefine a const variable, they wont.

3 Likes

That’s crazy! Based on the rules for how a constant is converted to match a result of an expression in a function. Very tricky. And why it’s very terrible indeed to change constants.

I won’t be changing these. The purpose is to define semantic indices into arrays.

Rather than saying mild_evolve_probability[5, 1], code is clearer by saying mild_evolve_probability[to_severe, a1], where a1 is agegrp 1.

Is there a better suggestion other than making these constant integers? They surely will not be changed as code will break. This is a case where it would be nice if there were a constant const or an alias. I suppose they can just be
globals but that seems to have a performance impact.

I could put these (there are a bunch) in a struct and pass around the struct and use them as idx.to_severe. I can type 4 more characters because my editor types them for me anyway. They wouldn’t need to be global (so they could be changed,
but not going to do that). But, the struct would need to be input to most functions, which is why globals used in this careful-ish way seemed sensible.

But, it is a terrible idea generally to change the value of a constant except potentially in the case of a non-constant element of a composite type.

Sure, I know that. I was just letting you know that you could change the value but not the type.

If you are consistently accessing specific elements of a vector or matrix by semantic labels, then I find that is often a sign that you need a different data structure. For example, perhaps what you actually have is something better represented by a DataFrame or an AxisArray or even just a Vector{MyStruct} where MyStruct is a (potentially mutable) struct with semantically labeled fields.

In other words, if you are using constant global variables to define offsets into blocks of memory, then you are essentially doing by hand what struct already does for you.

4 Likes

Thanks. This is a bit more complicated than some columns in a dataframe. Rows, columns, planes, hypercubes all have semantic references. It’s just 4 dimensions and it makes reading/updating very simple. A struct could help keep some
of these things “together.” The four dimensions are medical condition (8 items), age groups (5), lag days (19), locales (3651). We’ll see if I really use that many locales.

I’ll look at AxisArray. Thanks.