MRC-CSO-SPHSU / LoneParentsModel.jl

An initial implementation of an ABM for social and child care
0 stars 4 forks source link

(DON'T MERGE!) make lpm independent of MultiAgent.AbstractAgent #55

Closed mhinsch closed 2 years ago

mhinsch commented 2 years ago

DON'T MERGE!

just to show the basic idea

AtiyahElsheikh commented 2 years ago
mhinsch commented 2 years ago

Sorry about yearly2monthly, that wasn't supposed to be part of that patch (but yes, I'm sure that that's the right way to do it).

Concerning the other points - we can still keep the modularity, we would just have to declare the Demography module in LPM.jl/MALPM.jl. It's not pretty (as it logically belongs to demography), but I don't see how we would solve the AbstractAgent problem otherwise. Unless we want to do macro trickery, the place where the agent types are included has to be the place where either MultiAgents.AbstractAgent is included or a local dummy type declared instead.

mhinsch commented 2 years ago

One more point. I think I understand what you are getting at with the Demography module and I totally agree that it is a good idea to keep the various modules (Create, etc.) in a separate directory demography. I wonder though, if a module Demography is really needed? Will it ever happen that a simulation uses more than one of these modules at the same time? Anyway, I can put the module back in (just declared locally in MALPM.jl/LPM.jl) for now and we can think about whether we need it or not further down the line.

AtiyahElsheikh commented 2 years ago

Regarding Demography module, would it be always obvious that a certain function createF*()belongs to a certain module X? probably not. Moreover, there is a mapping between the module name, e.g.Demography and the folder name demography.

In the main code, it is nice to see something like that

using X.Create: ... 
using Y.Create: ...
... 
mhinsch commented 2 years ago

Ok, I was exceptionally stupid and didn't understand the actual problem. Turns out the solution is very simple - just have two versions of XAgents.jl in two different directories and push one or the other into LOAD_PATH dependent on which model version you want to use. This also means that Demography can happily remain in lpm/malpm. I'll close this pull request and apply the change directly to development.

(BTW, generally it might be helpful to transition to relative using, i.e. using statements that include the relative path (there are various ways to do that). If we had had that in place it would have been immediately obvious what the problem was and how to solve it.)