Improve xchr function(AlphaGenes/AlphaPeel#225)#226
Improve xchr function(AlphaGenes/AlphaPeel#225)#226AprilYUZhang wants to merge 9 commits intoAlphaGenes:develfrom
Conversation
Devel work from @XingerTang @AprilYUZhang and @RosCraddock and me!
|
@AprilYUZhang how do these changes impact the example results - can you share like I did at #199 (comment)? |
| if peelingInfo.isXChr: | ||
| if ind.sex == 0: | ||
| # male the segregation probabilities are 0.5 for pp and pm | ||
| peelingInfo.pointSeg[ind.idn, 0, :] = 0.5 - 1e-9 |
There was a problem hiding this comment.
@XingerTang can you suggest what we do here. @AprilYUZhang says that if she puts 0 in the 3rd andf 4th columns she gets division by zero, hence NaN. So, this 1e-9 is another error in peeling. Should we give this one a specific name and store it somewhere as a variable, so it doesn't get lumped into other error(s)? What are your thougts?
Thanks, gg;)
There was a problem hiding this comment.
@AprilYUZhang, I didn't experience the division by zero when I replace the value by zeros in your code and test with tests/functional_tests/run_func_test.py::TestClass::test_sex. Could you give me an example when this value could cause division by zeros? Thanks.
|
Hi @AprilYUZhang, can you provide a tinyhouse reference that this code can run with? |
|
e8a7cd3e12378941e2aef5a05ff161eb49204a9f |
…rove-xchr-function Merge latest devel branch
|
I have merged the latest devel branch, and it works. |
| maternalPattern <- matrix(0, nrow = 4, ncol = nLociAll) | ||
| for (comb in (1:nMaternalComb)) { | ||
| start <- indRecHist[[1]][comb, 2] | ||
| if (start < nLociAll+1){ |
There was a problem hiding this comment.
Nest the code, otherwise it's really hard to read
There was a problem hiding this comment.
I mean, since you added this if statement here. The code below/within the statement should be moved to the right.
| paternalPattern <- matrix(0, nrow = 4, ncol = nLociAll) | ||
| for (comb in (1:nPaternalComb)) { | ||
| start <- indRecHist[[2]][comb, 2] | ||
| if (start < nLociAll+1){ |
Related Issue
Relate to #225
What changed
Why this change
See #199 #216 #217
Notes / Risks
For x chr, the default seg probs are changed.