petrelharp / ftprime_ms

4 stars 2 forks source link

Update simplify figure and benchmarks. #58

Closed jeromekelleher closed 6 years ago

jeromekelleher commented 6 years ago

simplify-results

ashander commented 6 years ago

LGTM. Except panel labels of the sort shown here https://stackoverflow.com/a/18346779/4598520 (outside upper left corner of the panel) look better to me

ashander commented 6 years ago

Oh and in #20 you'd mentioned putting more lines on (A) but I don't think that's necessary for now

jeromekelleher commented 6 years ago

LGTM. Except panel labels of the sort shown here https://stackoverflow.com/a/18346779/4598520 (outside upper left corner of the panel) look better to me

I agree... the journal will change these anyway though, most likely, so there's not much point in getting too deep into it. Easiest to just keep it simple for now.

Oh and in #20 you'd mentioned putting more lines on (A) but I don't think that's necessary for now

(A) looks good to me now. We're seeing the variance pretty clearly, and the mean is also pretty obvious. No point in putting too many lines on I think.

ashander commented 6 years ago

Sounds good for both.

On the labels that's very true about production. I googled mostly b/c I was curious how hard it is to do such labels in pyplot or whatever. In R I'm spoiled by https://cran.r-project.org/web/packages/cowplot/vignettes/introduction.html

On Thu, Jan 11, 2018 at 9:48 AM, Jerome Kelleher notifications@github.com wrote:

LGTM. Except panel labels of the sort shown here https://stackoverflow.com/a/18346779/4598520 (outside upper left corner of the panel) look better to me

I agree... the journal will change these anyway though, most likely, so there's not much point in getting too deep into it. Easiest to just keep it simple for now.

Oh and in #20 https://github.com/petrelharp/ftprime_ms/issues/20 you'd mentioned putting more lines on (A) but I don't think that's necessary for now

(A) looks good to me now. We're seeing the variance pretty clearly, and the mean is also pretty obvious. No point in putting too many lines on I think.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/petrelharp/ftprime_ms/pull/58#issuecomment-357006620, or mute the thread https://github.com/notifications/unsubscribe-auth/AAfLONfT8tfugivwsPibyHGUQIBRN2mXks5tJkl1gaJpZM4RbFj8 .

petrelharp commented 6 years ago

nice, thanks for fixing up my hack-ey figures.