petrelharp / treestats_ms

1 stars 1 forks source link

Final pass from JK #29

Closed jeromekelleher closed 5 years ago

jeromekelleher commented 5 years ago

Here's some more edits from me. Comments below to explain why I've made them.

jeromekelleher commented 5 years ago

Still working on this, will update in a bit.

jeromekelleher commented 5 years ago

OK, done here. I think we can improve the flow of the end of the introduction into the next section; I'll follow up with some edits on that later.

jeromekelleher commented 5 years ago

This is ready for review and merge, I think.

petrelharp commented 5 years ago

This still true? You will do subsequent edits on another PR?

petrelharp commented 5 years ago

I've gone through these; some comments.

jeromekelleher commented 5 years ago

This still true? You will do subsequent edits on another PR?

Yeah, it would be good if you could take a quick pass through today if you can, there's a couple of issues outstanding.

jeromekelleher commented 5 years ago

Some more updates here. I've moved around some chunks of text, so best merge this and work from there if you're planning on doing any updates. I'm planning on making some more changes to the intro and discussion though, so might be best to wait on that.

jeromekelleher commented 5 years ago

Probably tomorrow when I get to this though: this conference is really good!

petrelharp commented 5 years ago

great! i'll wait.

On Tue, Sep 17, 2019 at 8:19 AM Jerome Kelleher notifications@github.com wrote:

Probably tomorrow when I get to this though: this conference is really good!

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/petrelharp/treestats_ms/pull/29?email_source=notifications&email_token=AAH7N2JKSB6R7AGUJWYX5FTQKDYQNA5CNFSM4IXAMFKKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD644SEY#issuecomment-532269331, or mute the thread https://github.com/notifications/unsubscribe-auth/AAH7N2L7GJ4CA3TTUQOYHCTQKDYQNANCNFSM4IXAMFKA .

jeromekelleher commented 5 years ago

OK, definitely done now @petrelharp! I've ended up changing a fair bit, but it's nearly all just moving stuff around. Hopefully an improvement, anyway.

What I'd suggest we do is merge this much (I won't be back online until tomorrow), and hopefully you can make a quick pass through @petrelharp. Hopefully we'll be basically ready to submit then, barring some final tweaks.

One thing: I'm not entirely happy with paragraph 2 of the intro. This can be done better, I'm sure. Feel free to rewrite it if you don't like it!

petrelharp commented 5 years ago

I've looked at all the changes; they look like just what we need! Excellent. I'll have a pass today sometime.

molpopgen commented 5 years ago

Let me know when to take another read. About to leave for the this school retreat in the desert, but should be able to carve out time.

petrelharp commented 5 years ago

Do you want to read it first? I have some syllabus-making to do before I get into this.

On Wed, Sep 18, 2019 at 9:22 AM Kevin R. Thornton notifications@github.com wrote:

Let me know when to take another read. About to leave for the this school retreat in the desert, but should be able to carve out time.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/petrelharp/treestats_ms/pull/29?email_source=notifications&email_token=AAH7N2PCBQFKCYAMWIUAGU3QKJITNA5CNFSM4IXAMFKKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD7AUNFQ#issuecomment-532760214, or mute the thread https://github.com/notifications/unsubscribe-auth/AAH7N2NXGHEROD5CZACVBBDQKJITNANCNFSM4IXAMFKA .

molpopgen commented 5 years ago

That could happen. I should be arriving at my destination in about 4 hours give or take. I'll get back to you...

On Wed, Sep 18, 2019, 9:34 AM Peter Ralph notifications@github.com wrote:

Do you want to read it first? I have some syllabus-making to do before I get into this.

On Wed, Sep 18, 2019 at 9:22 AM Kevin R. Thornton < notifications@github.com> wrote:

Let me know when to take another read. About to leave for the this school retreat in the desert, but should be able to carve out time.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub < https://github.com/petrelharp/treestats_ms/pull/29?email_source=notifications&email_token=AAH7N2PCBQFKCYAMWIUAGU3QKJITNA5CNFSM4IXAMFKKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD7AUNFQ#issuecomment-532760214 , or mute the thread < https://github.com/notifications/unsubscribe-auth/AAH7N2NXGHEROD5CZACVBBDQKJITNANCNFSM4IXAMFKA

.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/petrelharp/treestats_ms/pull/29?email_source=notifications&email_token=ABQ6OH7RPBYBFWIBYFFWNDTQKJJYRA5CNFSM4IXAMFKKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD7AVKBY#issuecomment-532763911, or mute the thread https://github.com/notifications/unsubscribe-auth/ABQ6OH5OZBEF72LZMG2TRHLQKJJYRANCNFSM4IXAMFKA .

petrelharp commented 5 years ago

Let me know so we don't end up doing it in parallel...

On Wed, Sep 18, 2019 at 10:04 AM Kevin R. Thornton notifications@github.com wrote:

That could happen. I should be arriving at my destination in about 4 hours give or take. I'll get back to you...

On Wed, Sep 18, 2019, 9:34 AM Peter Ralph notifications@github.com wrote:

Do you want to read it first? I have some syllabus-making to do before I get into this.

On Wed, Sep 18, 2019 at 9:22 AM Kevin R. Thornton < notifications@github.com> wrote:

Let me know when to take another read. About to leave for the this school retreat in the desert, but should be able to carve out time.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <

https://github.com/petrelharp/treestats_ms/pull/29?email_source=notifications&email_token=AAH7N2PCBQFKCYAMWIUAGU3QKJITNA5CNFSM4IXAMFKKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD7AUNFQ#issuecomment-532760214

, or mute the thread <

https://github.com/notifications/unsubscribe-auth/AAH7N2NXGHEROD5CZACVBBDQKJITNANCNFSM4IXAMFKA

.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub < https://github.com/petrelharp/treestats_ms/pull/29?email_source=notifications&email_token=ABQ6OH7RPBYBFWIBYFFWNDTQKJJYRA5CNFSM4IXAMFKKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD7AVKBY#issuecomment-532763911 , or mute the thread < https://github.com/notifications/unsubscribe-auth/ABQ6OH5OZBEF72LZMG2TRHLQKJJYRANCNFSM4IXAMFKA

.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/petrelharp/treestats_ms/pull/29?email_source=notifications&email_token=AAH7N2LWHPA3BMBK2S7OHVDQKJNSRA5CNFSM4IXAMFKKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD7AYLSQ#issuecomment-532776394, or mute the thread https://github.com/notifications/unsubscribe-auth/AAH7N2JB3ECP4APUH4SQW43QKJNSRANCNFSM4IXAMFKA .

molpopgen commented 5 years ago

ok, I am here, and the wifi is working. Gonna pull @jeromekelleher's branch now to read.

molpopgen commented 5 years ago

One thing I noticed is that @apragsdale's comments (#27) about the use of $\frac{1}{L}$ may still be confusing. If you look at the first few equations describing node stats (1-3), this term is in all of them. Then, the site stats all talk about specific genomic windows. While it is, or should be, clear from Eqn 4 that site stats are normalized by the length, the notation switch may be confusing?

I'm not entirely sure if this is a problem and, if it is, what the solution is. The node stats are introduced as a description of how much sample ancestry comes from what node, averaged across trees. But the site stats have a different motivation.

molpopgen commented 5 years ago

I have made a few edits, which are submitted as a PR upstream to @jeromekelleher's branch. I won't have time for much more today.

petrelharp commented 5 years ago

While it is, or should be, clear from Eqn 4 that site stats are normalized by the length, the notation switch may be confusing?

I suppose the right solution is to define Node statistics in windows, so the normalization is by 1/(j-1) instead of 1/L. I'll do this.