jasmineRepo / JAS-mine-core

JAS-mine maintains and develops the JAS simulation platform, a discrete-event tool-kit for agent-based and dynamic microsimulation modelling. This repository contains the core libraries. See www.jas-mine.net for more details.
2 stars 5 forks source link

Default alignment behaviour #28

Open vkhodygo opened 2 years ago

vkhodygo commented 2 years ago

This is addressed to you @pbronka

What's the best default behaviour of all alignment methods when the filtering method returns an empty list? Is it supposed to be an early exit point or we need to throw a new exception?

pbronka commented 2 years ago

Just to clarify, this would occur if one tried to align the share / probability of an empty cell? (For example, I want to align the employment rate of males aged 20 in the simulation, but there are no such individuals in the simulation?)

vkhodygo commented 2 years ago

I wouldn't put it like that, but close enough. We filter the whole population to pick only individuals that satisfy certain conditions. Sometimes, there are no such people meaning there is no need to run the whole alignment procedure.

It's either we do an early return and keep the simulation going further or we terminate via throw new. Since the same procedure of filtering is now used everywhere, all alignments should follow the same convention.

pbronka commented 2 years ago

Thanks. I'm just thinking through use cases where this would occur - trying to align an empty list doesn't sound like something that one would intend, so I think I'd be in favour of throwing an exception.

vkhodygo commented 2 years ago

That's the point: if there is an empty list, we don't run the alignment procedure at all, we simply go to return. I mean, it's not like you check how many individuals each list contains every time. It could be one or two or so out of thousands which potentially affects the results as much as no people at all (unless they have astronomically high weights).

This discussion boils down to one thing, whether to consider this a corner case requiring manual intervention or not, but we've been effectively saying this all this time. Empty list is a valid input/output, still, I'd like to hear your final decision regarding that.

pbronka commented 2 years ago

I don't think I appreciate the problem enough without a more concrete example and it sounds like you know how to best handle this, so please feel free to go with the option you think is best.

if there is an empty list, we don't run the alignment procedure at all, we simply go to return.

If that's the current behaviour, sounds fine to me to keep it that way then.

vkhodygo commented 2 years ago

Fair enough.

@dkopasker I'd like to hear your thoughts regarding this issue. As someone who uses this regularly, would you like to stop the simulation running if there is no people to align (small sample size, too tight filtering conditions etc.)?

dkopasker commented 2 years ago

Continuing the simulation but with a warning message would be my preference. A warning message for low cell counts would also be helpful, plus some output to check the cell counts in all lists.

vkhodygo commented 2 years ago

Continuing the simulation but with a warning message

We have limited options here to be honest. This part of the code does not interact with GUI elements at all, I can only add a log message in the case you find final results looking odd and want to double check the whole simulation process.

A warning message for low cell counts

Same story here.

some output to check the cell counts in all lists

Could you please elaborate on this one?

dkopasker commented 2 years ago

Adding a log message is fine. My overall point was that it should be possible for the user to check if they suspect a problem after the simulation has completed. The log file would be the first place to check, if warnings are present they should be able to check a further document.

You mentioned earlier that "it's not like you check how many individuals each list contains every time". It may be useful to keep a record to enable such a check.

vkhodygo commented 2 years ago

OK, I'll fix that.