elissasoroj / shadie

GNU General Public License v3.0
11 stars 4 forks source link

Improve name and types for "selection" argument in wright_fisher #22

Closed eaton-lab closed 4 months ago

eaton-lab commented 5 months ago

The current default is selection: str = 'none'. If using this behavior, it would be preferable to set default as None, rather than as a string. But I think it would overall be preferable to be able to set this as a binary switch rather than categorical result.

Recommendation

model.reproduction.wright_fisher(fitness_affects_survival=True)

Other ideas

Another option is to allow the user to set selection on either stage but to raise a warning when they perform a simulation that will not match typical expectations. This is less preferable to me since the user


# default
model.reproduction.wright_fisher(fitness_affects_survival=True, fitness_affects_mating_success=False)

# alternative
model.reproduction.wright_fisher(fitness_affects_survival=False, fitness_affects_mating_success=True)

# raises a warning that no fitness affects will be applied
model.reproduction.wright_fisher(fitness_affects_survival=False, fitness_affects_mating_success=False)

# raises a warning that selection occurs twice, and so the results will deviate from WF expectations.
model.reproduction.wright_fisher(fitness_affects_survival=True, fitness_affects_mating_success=True)

``
elissasoroj commented 5 months ago

Originally, I had intended on having soft and hard selection as options only for WF models. Under this assumption, both modes would never have been allowed at the same time, justifying a single parameter. The default set to none was consistent with the expectation that users will mostly run WF models in shadie as their neutral control, and that allowing selection breaks WF assumptions.

However, it has become clear that hard and soft selection modes should be made available in all shadie models and that we would like the flexibility for users to specify one or both for those models. As such, I will change the selection argument to multiple parameters consistent with your recommendation:

# default
model.reproduction.bryophyte_dioicous(fitness_affects_spo_survival=True, 
                                      fitness_affects_spo_reproduction_success=False,
                                      fitness_affects_gam_survival=True,
                                      fitness_affects_gam_mating_success=False)

#alternative
model.reproduction.bryophyte_dioicous(fitness_affects_survival=False, 
                                      fitness_affects_spo_reproduction_success=True,
                                      fitness_affects_gam_survival=False, 
                                      fitness_affects_gam_mating_success=True)

# raises a warning that fitness effects are applied to different part of the spo and gam lifecycles
model.reproduction.bryophyte_dioicous(fitness_affects_spo_survival=False, 
                                      fitness_affects_spo_reproduction_success=True,
                                      fitness_affects_gam_survival=True, 
                                      fitness_affects_gam_mating_success=False)

model.reproduction.bryophyte_dioicous(fitness_affects_spo_survival=True, 
                                      fitness_affects_spo_reproduction_success=False,
                                      fitness_affects_gam_survival=False,
                                      fitness_affects_gam_mating_success=True)

# raises a warning that no fitness effects will be applied
model.reproduction.bryophyte_dioicous(fitness_affects_spo_survival=False, 
                                      fitness_affects_spo_reproduction_success=False,
                                      fitness_affects_gam_survival=False,
                                      fitness_affects_gam_mating_success=False)

# raises a warning that selection occurs twice within at least one life stage,
#and so the results will deviate from WF expectations.
model.reproduction.bryophyte_dioicous(fitness_affects_spo_survival=True, 
                                      fitness_affects_spo_reproductive_success=True)

However, I will maintain the requirement that only one mode can be specified in WF models, at least until we see the need to allow for both. Allowing both will require more significant change to the code. I'm on the fence about what the default should be here, but still learning toward neutral as default.

# default (prints message that no fitness effects will be applied)
model.reproduction.wright_fisher(fitness_affects_survival=False, fitness_affects_mating_success=False)

#alternatives
model.reproduction.wright_fisher(fitness_affects_mating_success=True, fitness_affects_survival=False)
model.reproduction.wright_fisher(, fitness_affects_mating_success=False, fitness_affects_survival=True)

# raises an ValueError
model.reproduction.wright_fisher(fitness_affects_survival=True, fitness_affects_mating_success=True)

As an aside, one reason not to set the default fitness_affects_survival=True is because if the user then sets the mating success parameter, they will automatically be setting the model to double selection, but as long as we implement a logger warning I see no issues with this.

Finally, you seemed to be suggesting that allowing the user to set selection on either stage but to raise a warning is less preferable for some reason - or did you change your mind?

elissasoroj commented 4 months ago

Fixed in v1.0.1 release