search

gaynorr / AlphaSimR

R package for breeding program simulations
https://gaynorr.github.io/AlphaSimR/
Other
40 stars 17 forks source link

Consider having genParam write to popMisc #170

Open gaynorr opened 7 months ago

gaynorr commented 7 months ago

Is your feature request related to a problem? Please describe. Breeding values, deviations and variance components are population specific. The current strategy for handling them is for the genParam function to calculate them on a population when requested by the user and return the output as a fairly long list. There are several convenience functions for accessing portions of this output without explicitly calling genParam, such as bv and varA. They work by first calling genParam and then returning just the relevant information. If the user is using several of these functions, this results in multiple calls to genParam. The user would be better off calling genParam manually and extracting the relevant information manually, but this wouldn't be obvious to a user.

Describe the solution you'd like Storing the output to the popMisc slot could address the concern about the output being population specific, because the slot would be cleared if the population is changed. It would require running genParam on the population first to access the output, so it does make the convenience functions less convenient. This could be mitigated by having an option in SimParam that adds genParam to finalizePop. I'm not yet convinced that this is a better option, but putting it here to consider later.

Additional context This issue would be easier to address if populations were reference classes and not S4 classes, but this will require a significant change to the underlying code.

gregorgorjanc commented 7 months ago

Would this impact speed - if genParam are called every time a pop is crest or changed?

Maybe make it optional via a flag in SimParam?

gaynorr commented 7 months ago

Yes, simulations would be slower if it was ran automatically. Thus, I was thinking about having it off by default and having the option to turn it on. Although, I'm not sure this option is worthwhile when you could just have people run genParam when needed. Ideally, you'd be able to run varA(pop) and genParam gets triggered for pop automatically. However, I don't know how to make this work without converting populations to reference classes.