Inherited attributes

[jeromekelleher]

We've talked about how to deal with inherited attributes in a few places, so I thought I'd start an issue here to bring it together.

There's a tension between our goals of (a) providing a specification for humans to read that is non-repetitive; and (b) providing a specification for interchange that's as simple to implement as possible. For (a) we would definitely like to have inherited attributes, because we don't want to write down the same parameters over and again. For (b) we would kinda prefer not to have inherited attributes, because it does make the specification a bit more complex, and we would have to be quite precise about what demes parsers are expected to do.

So, here's a proposal. Our object tree currently looks like this:

root
+ --- description
+ --- demes
      + --- id
      + --- description
      + --- epochs
           + --- initial_size
           + --- final_size
+ --- pulses
     + --- source
     + --- dest
     + --- time
+ --- migrations
    + --- source
    + --- dest
    + --- rate

I don't like the idea of having things like cloning_rate defined as root attributes of the graph, because this implies that everything inherits this as a default (even pulses and migrations, say).

What if we added an explicit defaults section to the various levels of the tree, and the defaults are then given as fully qualified references? So, something like

description: An example demography to play around with cloning attributes.
generation_time: 1
time_units: generations
defaults:
    Epoch.cloning_rate: 0.05
    Epoch.initial_size: 1000

demes:
- id: pop1
    description: Population with epochs and changing cloning rates
    ancestors: root
    defaults:
         Epoch.cloning_rate: 0.1 # Overrides the default from higher in the tree
         Epoch.initial_size: 1e4
    epochs:
    -  end_time: 500
    - initial_size: 1e2
      end_time: 100
    - end_time: 0
      cloning_rate: 0.5
  - id: pop2
    description: Population with epochs and changing cloning rates
    ancestors: root
    epochs:
    - end_time: 500     
    - initial_size: 1e2
      end_time: 0
      cloning_rate: 1.0

So, the semantics can be defined precisely (if a value isn't given, go up the tree looking at the defaults, and if you don't find any, error out), and we're not cluttering up the namespace with things that aren't actually related to the object in question. The problem of outputting a compact/simplified (I'm on the fence about what the right terminology is here) is one of figuring out what can be put into the defaults to minimise repetition. This is a parsimony problem, which I'm sure we can solve.

The downside is that parsers have to be a bit more complicated, but I think this is something we can live with. So, to be clear, we get rid of the distinction between the low-level fully qualified graph that's used for interchange and settle on this single form which requires that implementations understand how default values work.

What do we think? I know I'm vacillating on what should and shouldn't go in the JSON schema definition (apologies!), but I feel like we're very close to something we can define precisely and start building on!

[grahamgower]

Nice idea! It does seem like this would clean a few things up.

[apragsdale]

I think containing those lower-level attributes within defaults is very nice - similar to when we started out with default_Ne, but this will contain anything that needs to be passed up to the deme/epoch levels.

You say that when we go up the tree looking at defaults and don't find any, it would error out - wouldn't it rather just set the attribute to None? I guess, do we allow demes where cloning_rate is given for some epochs but None in others, or that it's defined from some demes and not others?

[grahamgower]

You say that when we go up the tree looking at defaults and don't find any, it would error out - wouldn't it rather just set the attribute to None? I guess, do we allow demes where cloning_rate is given for some epochs but None in others, or that it's defined from some demes and not others?

We already have, e.g. selfing and cloning rates are 0 by default, so we don't have to error out unless that makes sense for that attribute.

[jeromekelleher]

Exactly yeah; I was more thinking about stuff that we need values for, like initial_size or whatever. We'd need to be careful about how this would interact with the default values defined at the leaf-level in the schemas too btw - it may be better to not set any default there and instead have defaults/Epoch.cloning_rate default to 0 instread, so that we're only setting a default in one place. It's a minor detail, though.

[jeromekelleher]

The main question is, how does this interact with the logic for deriving epoch start/end times? Is that something we should specify explicitly as well, or is it too complicated to expect it to be implemented identically more than once?

[apragsdale]

That all makes sense about the defaults of cloning and selfing rates set to zero already. Agree we probably want to only set it in one place.

The main question is, how does this interact with the logic for deriving epoch start/end times? Is that something we should specify explicitly as well, or is it too complicated to expect it to be implemented identically more than once?

I'm thinking that we probably don't want to have default start and end times for epochs specified at levels higher than the deme. I can only imagine corner cases where it's useful, and most cases where it would just make specifying a valid demography more difficult.

[grahamgower]

We'd need to be careful about how this would interact with the default values defined at the leaf-level in the schemas too btw - it may be better to not set any default there and instead have defaults/Epoch.cloning_rate default to 0 instread, so that we're only setting a default in one place.

Those "default" fields in the schema can easily be removed/changed. They don't actually do anything anyway, as far as I can tell. Some schema validators set those fields during validation. Validation of input models via the schema is probably unhelpful anyway, because it's nowhere near strict enough for our purposes (we can't specify inter-epoch constraints), and the error messages would be horrible for users.

The main question is, how does this interact with the logic for deriving epoch start/end times? Is that something we should specify explicitly as well, or is it too complicated to expect it to be implemented identically more than once?

I'm thinking that we probably don't want to have default start and end times for epochs specified at levels higher than the deme. I can only imagine corner cases where it's useful, and most cases where it would just make specifying a valid demography more difficult.

Yeah, the defaults mechanism described in this issue don't make sense for start/end times of epochs at all. So we'd need to have a way of "opting out" from the defaults propagation.

[jeromekelleher]

Agreed, start/end times are different and shouldn't have defaults. We can easily specify this in the schema, so that we only accept a fixed set of keys in the defaults sections

[grahamgower]

Thinking about this more, I'm not so sure it makes much headway into unifying a spec for simplified and fully-qualified graphs. The allowable fields in either spec are not so challening to define in a unified way (defaults or no defaults, the simplified spec would be a superset of the fully-qualified spec), the problem is one of clearly describing the semantics for omitted details in simplified graphs (and a json schema cannot describe object relations).

• Epoch start_time and end_time have their own defaults. And how these defaults are obtained depend on ancestor properties, and the adjacent epoch in the deme's epoch list (if any).
• Migration start_time and end_time have defaults defined by the time-intersection of the demes to which migration applies.
• Epoch size_function is "constant" by default, unless initial_size and final_size differ, in which case the default is "exponential".
• Epoch initial_size defaults to the previous epoch's final_size.
• Epoch final_size defaults to the same epoch's initial_size.

As far as I can tell, the only sensible things to include in defaults sections would be epochs.cloining_rate, epochs.selfing_rate, and epochs.initial_size (possibly complicated by the above rule). One could maybe make corner case arguments for migrations.rate and demes.ancestors, but I doubt they're much use in practice. It seems what we really want, is the ability to reference variables. But then we'd probably recommend to instead construct the graph via the python API, where variables are already available, and then dump to a file if that's really needed.

[jeromekelleher]

Good points @grahamgower. Maybe it would be productive to have 1/2 hour meeting on this next week, and save a bit of back and forth here?

[grahamgower]

Sure, happy to have a chat to work through the details. Next week is wide open for me.

[grahamgower]

We didn't really resolve whether to include a defaults section... But I don't think its needed if we're only using it for the selfing and cloning rates?

[jeromekelleher]

I think it would still be clearer if we did have a defaults section, and we called the values something like Epoch.cloning_rate. It just seems confusing to me to drop a bunch of stuff into the top-level, and we'll surely end up with more things over time.

[jeromekelleher]

I guess we should move this to the spec repo?

[grahamgower]

I wonder if the defaults section proposed here doesn't fit with cloning/selfing rates either. @apragsdale mentioned phylogenetic inheritance of these properties in the last call, which would make more sense than some kind of graph-wide or even deme-wide default value. Consider other things people might want to put in a defaults section, like ploidy, variance in number of offspring, or maybe height. Even when they're truly life-cycle characteristics (i.e., not height), I think such things ought to be explicitly modeled as heritable traits. This is in stark contrast to population size, which we do not model as heritable.

[jeromekelleher]

Definitely open to suggestions here @grahamgower - my main point is that we shouldn't define default values directly as attributes of the graph when they only apply to a subset of the entities defined in the Graph.

[grahamgower]

Sure, I certainly agree with that. But I wonder whethere we ever want defaults like this, or if all defaults we can think of are actually heritable, or otherwise need some very custom semantics. So either we come up with some general way of specifying heritable traits, or we say that we're not doing that at all (at least not now).

Maybe @apragsdale has some insight here about what he wanted to do with the selfing and cloning rates?

[jeromekelleher]

So either we come up with some general way of specifying heritable traits, or we say that we're not doing that at all (at least not now).

Agreed - leaning towards the latter!

[apragsdale]

So either we come up with some general way of specifying heritable traits, or we say that we're not doing that at all (at least not now).

Agreed - leaning towards the latter!

Also agree. I brought that up that idea of traits being inherited from ancestors, but I don't really like it. I think it makes more sense to have defaults specified in some defaults section at the graph (i.e. "species") level, as @jeromekelleher suggested, and then those are assigned to demes/epochs if they are not specifically given to a deme or epoch.

I don't know if the defaults should be Epoch.cloning_rate, Deme.cloning_rate or just cloning_rate - though in the last case it doesn't do much to say where the default value would be applied. Because you could also imagine placing something like Migration.rate in the defaults section as well. So I'm leaning toward Deme.cloning_rate... Instead of Epoch.cloning_rate, since rates are properties that can be specified at the deme level, and then get passed to each epoch based on the value at the deme level. And if an epoch doesn't have a cloning rate specified, it first looks to the deme level before looking in the defaults section.

[jeromekelleher]

I don't know if the defaults should be Epoch.cloning_rate, Deme.cloning_rate or just cloning_rate - though in the last case it doesn't do much to say where the default value would be applied. Because you could also imagine placing something like Migration.rate in the defaults section as well. So I'm leaning toward Deme.cloning_rate... Instead of Epoch.cloning_rate, since rates are properties that can be specified at the deme level, and then get passed to each epoch based on the value at the deme level. And if an epoch doesn't have a cloning rate specified, it first looks to the deme level before looking in the defaults section.

What about #25 though?

[apragsdale]

Even if we enforce specifying epochs for each deme, I think we still want to allow specifying inherited properties at the deme level. You could want to write a deme with a bunch of epochs with size changes, but the selfing rate is constant across all of them. You'd have to repeat the selfing_rate in each epoch then, which will lead to mistakes and we want to avoid.

I think the hierarchy of inheritance would look like:

1. if deme selfing rate is None, look in graph defaults and assign to deme if specified there
2. if epoch selfing rate is None, look in deme defaults, which has either been given or set by the graph default

and that's not too overly complicated?

[jeromekelleher]

What about Demes etc having a defaults section where you can specify these things? It would be really nice if the properties we define on objects were actual attributes of the thing in question (other than the defaults section).

[jeromekelleher]

Sorry, this is what you were suggesting! No, looking up the document tree for values is totally fine imo

[apragsdale]

I see - yes that makes sense. So something like this?

defaults:
- selfing_rate: 0.1
demes:
- id: deme1
  epochs:
  - ...
  defaults:
  - selfing_rate: 0.2
- id: deme2
  epochs:
  - ...

where the epochs in deme1 would set selfing rate to 0.2 if not given within the epoch, but the epochs in deme2 would grab the default of 0.1 if not given within the epochs.

[grahamgower]

Also agree. I brought that up that idea of traits being inherited from ancestors, but I don't really like it. I think it makes more sense to have defaults specified in some defaults section at the graph (i.e. "species") level, as @jeromekelleher suggested, and then those are assigned to demes/epochs if they are not specifically given to a deme or epoch.

@apragsdale, what exactly did you have in mind with the selfing and cloning rates where you don't want trait heritablility?

Just so that we don't rule this out completely, I was suggesting that we say "these traits are inherited", then we don't do anything at all in the spec or the python implementation, other than saying that. It's up to simulators to apply a trait value when it's specified (presumably at the roots), provide a sensible default if it's not specified, and propagate it through to descendants via splits, branches, mergers, admixtures, pulses and migrations. One can change the trait value at some internal part of the graph by setting a value in the relevant deme in the relevant epoch. The rule(s) for how any particular trait is inherited and propagated is up to the simulator.

[apragsdale]

@apragsdale, what exactly did you have in mind with the selfing and cloning rates where you don't want trait heritablility?

I think there are too many corner cases and ambiguities with passing a selfing rate from an ancestor deme to descendants (and what about two ancestors that have different selfing rates? which takes precedent?). I think it's cleaner to just have graph ("species") defaults and demes defaults, which are much cleaner to both describe and implement. Would you agree?

Just so that we don't rule this out completely, I was suggesting that we say "these traits are inherited", then we don't do anything at all in the spec or the python implementation, other than saying that. It's up to simulators to apply a trait value when it's specified (presumably at the roots), provide a sensible default if it's not specified, and propagate it through to descendants via splits, branches, mergers, admixtures, pulses and migrations. One can change the trait value at some internal part of the graph by setting a value in the relevant deme in the relevant epoch. The rule(s) for how any particular trait is inherited and propagated is up to the simulator.

I think this is a bit dangerous, because it means that two simulators look at the same YAML specification but then simulate under different rules. I think we want to determine the rules of how properties are inherited within demes and not leave it up to the simulators on how to propagate them. If, for example, msprime decides to pass inherited attributes differently than slim, leading to different behavior that is hidden from the user, it could lead to errors and confusion for users.

[grahamgower]

Those are very good points, and I agree that having defaults are easier to define clear behaviour for, and there's negligible burden for us to implement these defaults. But if demes becomes as widespread as we hope, then I expect extensions will profilerate, and I worry that other people will use the defaults section for clearly heritable traits.

[jeromekelleher]

Should we add an example trait that is clearly heritable then @grahamgower, just to make the framework for future extensions clear?

[grahamgower]

Should we add an example trait that is clearly heritable then @grahamgower, just to make the framework for future extensions clear?

That's an interesting idea. The choice of example trait would want to be 100% heritable, and clearly quantitative, or clearly Mendelian. But now that you've got me thinking about this, I admit that supporting heritable traits is probably a huge can or worms. Probably it would be best for the user to specify Mendelian or quantitative. And a trait could instead require very specialised semantics (e.g. ploidy in plants); it could be partially heritable; it could have an interaction with a per-deme environmental attribute. Users will want multiple "traits" to emulate SNVs that are (partially) linked, and then we start talking contigs/chromosomes...

So, sorry for derailing the discussion. Lets do the defaults thing.

[grahamgower]

I see - yes that makes sense. So something like this?

defaults:
- selfing_rate: 0.1
demes:
- id: deme1
  epochs:
  - ...
  defaults:
  - selfing_rate: 0.2
- id: deme2
  epochs:
  - ...

where the epochs in deme1 would set selfing rate to 0.2 if not given within the epoch, but the epochs in deme2 would grab the default of 0.1 if not given within the epochs.

Lets do exactly this, and without saying Deme.selfing_rate or Epoch.selfing_rate. At the risk of user confusion, I think we want to limit this to selfing_rate and cloning_rate too.

[jeromekelleher]

OK, sounds good to me. What's the dislike for the fully-qualified object references in the defaults section though, out of interest? I thought it would be a good idea to future-proof things a bit in case we wanted to do something like adding default migration rates (where the name of the property is just rate and therefore quite confusing to include at the top level defaults).

[grahamgower]

What's the dislike for the fully-qualified object references in the defaults section though, out of interest?

The full path to selfing_rate is demes[j].epochs[k].selfing_rate for the k'th epoch of deme j. For the defaults section to be usable, we'd have to at the least remove the [j] and [k]. Ok, not so bad. But the toplevel defaults is really setting the deme defaults, so what now? Well, the toplevel defaults would then more correctly say demes.selfing_rate and the defaults section inside a specific deme would be epochs.selfing_rate. Is that really better? And should these prefixes really be plural, or singular?

[jeromekelleher]

Well I was thinking of using the "class" reference here to avoid that - Epoch.selfing_rate is a fully qualified reference to the attribute.

[grahamgower]

But that has two problems. First, when defaults are set at the toplevel, this actually sets the deme default, not the epoch default. Second, the class name is an implmentation detail of the current parser, which is in no way necessary. E.g. an implemntation could just keep everything as the nested dicts/lists object like that obtained from json.load().

[jeromekelleher]

I thought we were going to make selfing_rate an attribute of Epoch though, weren't we? Anything that can change epoch-by-epoch should be an attribute of Epoch for simplicty.

I think it's entirely natural for us to say within the spec that the JSON objects correspond to different classes - it would be quite awkward and difficult not to, I would imagine.

[grahamgower]

I thought we were going to make selfing_rate an attribute of Epoch though, weren't we? Anything that can change epoch-by-epoch should be an attribute of Epoch for simplicty.

Yep, that's definitely what we have now, and I don't see that changing. So naming selfing_rate as epochs.selfing_rate would be fine then. Or should it be demes.epochs.selfing_rate at the top level?

I think it's entirely natural for us to say within the spec that the JSON objects correspond to different classes - it would be quite awkward and difficult not to, I would imagine.

It's just a data model, why does there need to be any mention of classes? That sounds like an OO language implementation detail. I find the two different capitalisations within the yaml file is confusing.

[jeromekelleher]

Sure, it's just a data model, and OO is just an abstraction that's sometimes useful in modelling things. It's a fairly well known abstraction with cross language conventions, so I thought this would be a good way to disambiguate as well as making the specification more readable/concrete. Let's see what @apragsdale and @molpopgen think here.

[molpopgen]

Sure, it's just a data model, and OO is just an abstraction that's sometimes useful in modelling things. It's a fairly well known abstraction with cross language conventions, so I thought this would be a good way to disambiguate as well as making the specification more readable/concrete. Let's see what @apragsdale and @molpopgen think here.

I've read through this thread twice and I'm having trouble coming up with a "short version" of the question at hand?

[jeromekelleher]

The question is, which of the following versions so you think is better:

A:

defaults:
- selfing_rate: 0.1
demes:
- id: deme1
  epochs:
  - ...
  defaults:
  - selfing_rate: 0.2
- id: deme2
  epochs:
  - ...

B:

defaults:
- demes.epochs.selfing_rate: 0.1
demes:
- id: deme1
  epochs:
  - ...
  defaults:
  - epochs.selfing_rate: 0.2
- id: deme2
  epochs:
  - ...

C:

defaults:
- Epoch.selfing_rate: 0.1
demes:
- id: deme1
  epochs:
  - ...
  defaults:
  - Epoch.selfing_rate: 0.2
- id: deme2
  epochs:
  - ...

I'm not so keen on A because we can imagine having a parameter named X sometime in the future that is in both (say) SymmetricMigration and in an Epoch. B and C are suggestions to disambiguate this. C is my suggestion where we disambiguate a field by prefixing it with its "class" name, with the understanding that we use a simple language independent object model as our framework for describing the entities in the model and their relationships. Graham isn't so keen on this, as he (reasonably) views the object oriented stuff as unnecessary.

[apragsdale]

I agree here that we want to avoid A, for future-proofing like you all have pointed out. I'm a bit torn between B and C here, but I think C is a bit cleaner. There isn't really any ambiguity about where epochs are found (only within demes), so I don't know if we need the demes prefix, and could just have epochs.selfing_rate at the top level as well? I agree we need something though, because we will want to have the flexibility to place migrations.rate into the defaults at the top level or the migrations level.

I do agree with Graham that the mixing of upper and lower case is a bit jarring perhaps. I don't know if we want to be flexible and allow either epochs.selfing_rate or Epochs.selfing_rate within the defaults? Probably not, and I think every-day users might get tripped up by having to place a capitalization where they don't expect to need it?

I'm going to wait until we iron out all the details here before finalizing the PR over in popsim-consortium/demes-python#172

[apragsdale]

Speaking of default migration rates, I could imagine something like a classical n-demes-in-a-line type model:

...
defaults:
- migrations.rate: 0.01
demes:
- id: deme_1
  epochs:
  - initial_size: 1000
...
- id: deme_n
  epochs:
  - initial_size: 1000
migrations:
- symmetric:
  - demes: [deme_1, deme_2]
  - ...
  - demes: [deme_{n-1}, deme_n]

or that defaults block within migrations:

migrations:
- defaults:
  - rate: 0.01
- symmetric:
  - demes: [deme_1, deme_2]
  - ...
  - demes: [deme_{n-1}, deme_n]

[grahamgower]

I'm not so keen on A because we can imagine having a parameter named X sometime in the future that is in both (say) SymmetricMigration and in an Epoch. B and C are suggestions to disambiguate this.

As a example, we already have proportions under demes and symmetric migrations. Ok, so wanting to set a default for either of these is unlikely to be useful. But you've convinced me that option A is a bad idea.

There isn't really any ambiguity about where epochs are found (only within demes), so I don't know if we need the demes prefix, and could just have epochs.selfing_rate at the top level as well?

That would work. It's effectively the same as saying Epoch.selfing_rate.

Speaking of default migration rates, I could imagine something like a classical n-demes-in-a-line type model:

<snip>

or that defaults block within migrations:

migrations:
- defaults:
  - rate: 0.01
- symmetric:
  - demes: [deme_1, deme_2]
  - ...
  - demes: [deme_{n-1}, deme_n]

I'd certainly encourage having the rate defined closer to where it applies, as that's clearer to me. But it probably doesn't matter much if both things are supported.

[jeromekelleher]

Great - being able to set default migration rates was just the sort of thing I was thinking about. So, the remaining question is: migrations.rate or Migration.rate?

...
defaults:
- migrations.rate: 0.01
- epochs.initial_size: 1000
demes:
- id: deme_1
...
- id: deme_n
migrations:
  - demes: [deme_1, deme_2]
  - ...
  - demes: [deme_{n-1}, deme_n]

or

...
defaults:
- Migration.rate: 0.01
- Epoch.initial_size: 1000
demes:
- id: deme_1
...
- id: deme_n
migrations:
  - demes: [deme_1, deme_2]
  - ...
  - demes: [deme_{n-1}, deme_n]

(I've simplified down the yaml here assuming we do #29; also, seems OK to implicitly define a single Epoch when using the default initial_size like this?)

There's not much in it. The only difference I can see is that we have two things called demes: the top level list of Deme definitions and the list of deme IDs involved in symmetric migrations.

[molpopgen]

I agree that B or C is the way to go. I'm even okay with C, using epoch instead of Epoch? At the level of the YAML, I'm not sure what the E gives us?

[jeromekelleher]

Sure, I just thought it was most natural to think of Epoch and Migration etc as classes so we'd adopt the standard naming convention around that for clarity. So, I think of it as Migration.rate: 0.01 is the default value for the attribute rate in the Migration class. But you're right, it makes no difference from a yaml/json perspective.

I don't feel strongly about it, happy to go with the consensus here.

[grahamgower]

@jeromekelleher, are you suggesting we should allow defaults for any property/attribute? I'm fairly sure we don't want to allow defaults for, say, Deme.id (they must be unique in a graph), and probably not for Epoch.start_time or Epoch.end_time (how defaults would interact with inheriting values from time-adjacent epochs would likely cause confusion). So do we have an explicit list of things that are allowed to have defaults? Or an explicit list of things that aren't allowed?

[jeromekelleher]

I think we need to have an explicit list of things that are allowed. I've been working on a clean and "obviously right" way to do the default value propagation in my #26 - it's surprisingly tricky.

[jeromekelleher]

While writing the code for #26 I figured out a reasonably simple way to propagate defaults, which currently will work for any property. Parsing out the prefix from e.g. Epoch.selfing_rate is annoying and ugly, though, so I have another slight tweak on the proposal. Instead of having things like Epoch.x: 1 and Epoch.y: 2 in the defaults list, we instead have a mapping epoch: {"x": 1, "y": 2}. This makes parsers a lot simpler, and also makes it easier to validate what should be allowed in the defaults sections of the different objects. So, concretely, we'd have (excuse my yaml, it's probably wrong!):

defaults:
  migration: 
      rate: 0.01
  epoch:
      initial_size: 1000
demes:
- id: deme_1
...
- id: deme_n
migrations:
  - demes: [deme_1, deme_2]
  - ...
  - demes: [deme_{n-1}, deme_n]

See what I mean?

[grahamgower]

Perfect!

[molpopgen]

Yeah, that's quite nice.