Stdpopsim: Implement checking sampling_time in model comparison

[ndukler]

Check that sampling properties are also the same when comparing that two models are equal in QC.

[jeromekelleher]

Has this been done @ndukler? Should be an easy enough change if not?

[ndukler]

It hasn't been done yet, I'll have it done by the end of the week

[ndukler]

So this is going to require augmenting all the qc models as none of them actually implement populations objects and as such have no actual sampling times. I think this should be pretty straightforward as all sampling times are either 0 or None right now but I'll mention if something comes up.

[ndukler]

Also this leads to the interesting feature where the get_samples() function doesn't work on the qc models.

[jeromekelleher]

How so?

[ndukler]

It requires populations to retrieve the sampling times and coughs up an error because there it can't find the attribute:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "C:\Users\ndukl_000\Documents\GitHub\stdpopsim\stdpopsim\models.py", line 294, in get_samples
    if self.populations[pop_index].allow_samples:
AttributeError: 'DenisovanAncestryInPapuans' object has no attribute 'populations'

[ndukler]

Do we want to add a default vector of populations at model creation or something to avoid this (I don't think we do)? It doesn't show up in testing because we never sample from the qc models.

[jeromekelleher]

Right, I see. We probably want a default list of populations all right.

[ndukler]

Ok, so I've done everything but we actually do have a disagreement in sampling times for the _papuans_10j19 model between the qc and production. The last four populations are non-sampling and the production model and 0 in the qc model. The 0 is clearly not the desired answer as they are archaic populations so I will set them to None for now unless there is any reason not to ?

[grahamgower]

Was this fully taken care of in #429? Can it be closed now?

[ndukler]

Yep