Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider having genParam write to popMisc #170

Open
gaynorr opened this issue Dec 23, 2023 · 2 comments
Open

Consider having genParam write to popMisc #170

gaynorr opened this issue Dec 23, 2023 · 2 comments

Comments

@gaynorr
Copy link
Owner

gaynorr commented Dec 23, 2023

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
Copy link
Contributor

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
Copy link
Owner Author

gaynorr commented Dec 24, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants