gaynorr / AlphaSimR

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

sortPed function bug #110

Closed AudreyAAMartin closed 1 year ago

AudreyAAMartin commented 1 year ago

Describe the bug The sortPed function should return a pedigree with information/column about which generation individuals are part of. In the current setting, all individuals are (wrongly) attributed gen 1.

Steps To Reproduce

Pedigree setting

id = 1:5 mother = c(0,0,0,1,1) father = c(0,0,2,0,3)

creating the ped and getting the gen information

ped = sortPed(id=id, mother=mother, father=father, maxCycle=100)

Expected behavior I would expect the founders to get a value of 1 for gen, and then the offspring to be of a later generation than their parents. For the pedigree used as example, I would expect this output: gen id mother father 1 1 0 0 1 2 0 0 2 3 0 2 2 4 1 0 3 5 1 3

gregorgorjanc commented 1 year ago

Just to add that the behaviour is

AlphaSimR:::sortPed(id=id, mother=mother, father=father, maxCycle=100)
  gen id mother father motherID fatherID
1   1  1     NA     NA        0        0
2   1  2     NA     NA        0        0
3   1  3     NA      2        0        2
4   1  4      1     NA        1        0
5   1  5      1      3        1        3

So all individuals have the same generation.

gregorgorjanc commented 1 year ago

This PR https://github.com/gaynorr/AlphaSimR/pull/111 proposes a fix

gaynorr commented 1 year ago

Thanks for fix @AudreyAAMartin.

AudreyAAMartin commented 1 year ago

@gaynorr for some reason the current CRAN version does not include the PR https://github.com/gaynorr/AlphaSimR/pull/111, despite the fact that the main branch has this commit accepted, which is very odd https://github.com/gaynorr/AlphaSimR/commit/2215f29d6df172868ad9f2a7fd69bf51f40f3a92.

Perhaps you need to rebuild and reupload the package to CRAN?

AudreyAAMartin commented 1 year ago

@gaynorr is it because your devel and main branches are not in sync?

Screenshot 2023-04-11 at 15 37 55
gaynorr commented 1 year ago

@AudreyAAMartin thanks for pointing this out. I forgot about this when submitting to CRAN, and submitted the source package from devel that didn't include your changes made on the master. I'll look into syncing these branches.

gaynorr commented 1 year ago

@AudreyAAMartin I merged the master branch into devel and that should sort out the issue. Your changes are now reflected in both branches and should go to CRAN with the next submission.

I tend to do all my work on devel and submit to CRAN for there. I initiate a pull request from devel to master when I'm getting ready submit and use GitHub actions to test the package before submitting. I've been trying to approve the pull request after the package is accepted on CRAN, so that the master version reflect what's on CRAN. However, I'm not always consistent with what I do.

It was my mistake for forgetting to merge your submission to devel before submitting.