Skip to content
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

Subtyping type parameterization does not work in @multiagent. #968

Closed
Datseris opened this issue Feb 4, 2024 · 13 comments · Fixed by #972
Closed

Subtyping type parameterization does not work in @multiagent. #968

Datseris opened this issue Feb 4, 2024 · 13 comments · Fixed by #972
Labels
agent-construction About making agents

Comments

@Datseris
Copy link
Member

Datseris commented Feb 4, 2024

MWE:

using Agents


@multiagent struct MultiSchelling{X<:Real}(GridAgent{2})
    @agent struct Civilian # can't re-define existing `Schelling` name
        mood::Bool = false
        group::Int
    end
    @agent struct Governor{X<:Real} # can't redefine existing `Politician` name
        group::Int
        influence::X
    end
end

erros with

ERROR: TypeError: in <:, expected Type, got a value of type TypeVar
Stacktrace:
 [1] top-level scope
   @ C:\Users\datse\.julia\dev\Agents\src\core\agents.jl:312

By the way, I find it odd that one must specify the type X in both the super type and the subtype. Can't the subtypes remain just a pure name ?

@Datseris Datseris added the agent-construction About making agents label Feb 4, 2024
@Tortar Tortar removed their assignment Feb 5, 2024
@Tortar
Copy link
Member

Tortar commented Feb 5, 2024

seems like a problem which should be addressed inside MixedStructTypes.jl

(don't really like assignments, but I will solve this soon anyway :D)

@Datseris
Copy link
Member Author

Datseris commented Feb 5, 2024

@Tortar before solving anything, can we first please consider a syntax simplification of the macro? Instead of

@multiagent struct MultiSchelling{X<:Real}(GridAgent{2})
    @agent struct Civilian # can't re-define existing `Schelling` name
        mood::Bool = false
        group::Int
    end
    @agent struct Governor{X<:Real} # can't redefine existing `Politician` name
        group::Int
        influence::X
    end
end

we could have

@multiagent struct MultiSchelling{X<:Real}(GridAgent{2})
    @subagent Civilian # can't re-define existing `Schelling` name
        mood::Bool = false
        group::Int
    end
    @subagent Governor # can't redefine existing `Politician` name
        group::Int
        influence::X
    end
end

two key differences: first, the subtypes do not need @agent struct, instead they get only @subagent. Only the supertype gets the struct to indicate more that this is the only actiual struct being created. Second, the subtypes do not get type parameterizations. That to me seems like a way to make the user make mistakes. There is no reason to duplicate the type parametetrization information, it should only be in the super type.

@Tortar
Copy link
Member

Tortar commented Feb 5, 2024

yes, I like it, but actually there is a reason, the subtypes are not functions, they are types themselves which accept constructor as Governor{Int}(...), if they would be just function it would have been impossible to have those constructors. But I don't think we need these explicit constructors in Agents.jl, and we can just live with Governor(1,...). Regardless, though, I think it is good to keep those constructors in the downstream package https://github.com/JuliaDynamics/MixedStructTypes.jl so if we want to go into this we should nonetheless reconstruct the parameter, in the sense that this

@multiagent struct MultiSchelling{X<:Real}(GridAgent{2})
    @subagent Civilian
        mood::Bool = false
        group::Int
    end
    @subagent Governor
        group::Int
        influence::X
    end
end

should become

@compact_struct_type MultiSchelling{X<:Real} begin
    mutable struct Civilian
        # fields of gridagent
        mood::Bool = false
        group::Int
    end
    mutable struct Governor{X} # -> again X here
        # fields of gridagent
        group::Int
        influence::X
    end
end

Seems not too difficult to check the presence of a parameter inside each subagent, so this should be doable

@Datseris
Copy link
Member Author

Datseris commented Feb 5, 2024

But I don't think we need these explicit constructors in Agents.jl, and we can just live with Governor(1,...).

Yes, I agree.

Why should we keep {X} inside the subtype? In the new tutorial I am telling the users "think of these subtypes as just functions that initialize the main type". The details of whether they actually are types or not are perhaps not useful for the end user.

@Tortar
Copy link
Member

Tortar commented Feb 5, 2024

No, I mean that it is fine not to have it in Agents.jl but I would really keep it in MixedStructTypes.jl which does the heavy lifting and so we need to reconstruct it anyway in the multiagent macro, but this is an implementation detail

@Tortar
Copy link
Member

Tortar commented Feb 5, 2024

Also actually we need to add a begin

@multiagent struct MultiSchelling{X<:Real}(GridAgent{2})
    @subagent Civilian begin
        mood::Bool = false
        group::Int
    end
    @subagent Governor begin
        group::Int
        influence::X
    end
end

but this seems anyway better

@Datseris
Copy link
Member Author

Datseris commented Feb 6, 2024

Actually , thinking what you said that Governor{X} is possible, perhaps we should leave the {X} there as well...?

@Tortar
Copy link
Member

Tortar commented Feb 6, 2024

yeah, I was in favor of changing this, because has some advantages stemming from the fact that we could really use function and not types for those names, which has some other consequences, but in reality after thinking about this I think that keeping the way it is now is better. So we are just left with the @subagent thing, I think that we agree that this change is an improvement instead, so in the end:

@multiagent struct MultiSchelling{X<:Real}(GridAgent{2})
    @subagent Civilian begin
        mood::Bool = false
        group::Int
    end
    @subagent Governor{X} begin
        group::Int
        influence::X
    end
end

@Datseris
Copy link
Member Author

Datseris commented Feb 6, 2024

okay, and what would happen if a usrer wrote Governor{X<:Real} when defining the macro? Like in #967 ?

@Datseris
Copy link
Member Author

Datseris commented Feb 6, 2024

Actually, now that I think about it, I am more in favor of not having {X} in the subagents and truly making the subagent names just functions. So the macro literally creates two functions:

function Governor(id, pos, group, influence)
return MultiSchelling( ...)
end

function Governonr(; id, pos, group = 1, influence = 1)
return MultiSChellng(...)
end

and we instruct the users to simply not put type annotations when calling the types. Generally speaking, there shouldn't be any reason to do this, to differentiate via the type parameterization. Right? Or can we imagine a scenario where this is important?

@Tortar
Copy link
Member

Tortar commented Feb 7, 2024

Tried to imagine something, it is not clear to me when this would be really needed, but in any case, even if we don't mention in the expression of the macro, it will be there anyway, at least for some time, just undocumented, and in general it won't never be really as simple as those functions you wrote, but I think that we don't need to specify everything in details to users

@Tortar
Copy link
Member

Tortar commented Feb 13, 2024

ok I actually understood when we really need to specify parameters e.g. with a normal struct

julia> struct A{T}
           x::Union{Int, T}
       end

julia> A(1)
ERROR: UndefVarError: `T` not defined
Stacktrace:
 [1] A(x::Int64)
   @ Main ./REPL[1]:2
 [2] top-level scope
   @ REPL[2]:1

julia> A{Int}(1)
A{Int64}(1)

same happen with MixedStructTypes.jl types

@Tortar
Copy link
Member

Tortar commented Feb 13, 2024

so the only thing we can really do is changing @agent in @subagent

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
agent-construction About making agents
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants