search

smithlabcode / preseq

Software for predicting library complexity and genome coverage in high-throughput sequencing.
https://preseq.readthedocs.io
GNU General Public License v3.0
78 stars 16 forks source link

Allow user to specify seed for RNG #26

Closed jstjohn closed 2 years ago

jstjohn commented 8 years ago

This is an untested change to allow the end-user to specify the seed for the RNG used by preseq. Do you have any tests that you typically run the various code through to verify correctness? It would be great to have a make test type rule at some point. Thanks!

jstjohn commented 8 years ago

We did some light testing insofar as we have verified that the results look similar between this and the master branch version, and the results do not appear to change between runs if you specify the -seed INT argument.

andrewdavidsmith commented 8 years ago

I'll try to look at this soon. Thanks!!!

Sent from my iPhone

On Sep 14, 2016, at 9:49 AM, John St. John notifications@github.com<mailto:notifications@github.com> wrote:

We did some light testing insofar as we have verified that the results look similar between this and the master branch version, and the results do not appear to change between runs if you specify the -seed INT argument.

You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://urldefense.proofpoint.com/v2/url?u=https-3Agithub.com_smithlabcode_preseq_pull_26-23issuecomment-2D247078347&d=DQMFaQ&c=clK7kQUTWtAVEOVIgvi0NU5BOUHhpN0H8p7CSfnc_gI&r=2ti3K9K1XjkPszCPHB82Aw&m=F5FhoeKlCIxIYbFnZDyZ4aHQnKCYB7F4B1Ouoc_4fT8&s=Jd6KqMRnGGPTZ0Ma_cpE_7vADSlKgakzWvNnUWHJyxA&e=, or mute the threadhttps://urldefense.proofpoint.com/v2/url?u=https-3Agithub.com_notifications_unsubscribe-2Dauth_AHBBgV-5FifNlA6Ipq6HKtVCX0DWrbMPhrks5qqCWigaJpZM4J8B-5FH&d=DQMFaQ&c=clK7kQUTWtAVEOVIgvi0NU5BOUHhpN0H8p7CSfnc_gI&r=2ti3K9K1XjkPszCPHB82Aw&m=F5FhoeKlCIxIYbFnZDyZ4aHQnKCYB7F4B1Ouoc_4fT8&s=jQ2X77B53sE65pK5XW36TBo8Bxs4ifnMTNVChw7XpTc&e=.

nboley commented 7 years ago

Any update on this?

timydaley commented 7 years ago

I updated preseq with a rng seed option that I had written in as part of https://github.com/timydaley/preseq_dev.
Thank you for the reminder, this had completely slipped my mind.

nboley commented 7 years ago

Hi Tim, thanks for looking at this. One question: If I understand the commit correctly, setting the seed to 0 with the command line option would actually result in a random seed being chosen. I think that this behavior is a bit unexpected -- the original PR got around this by initializing the seed to random and then allowing optparse to override this.

Do I understand correctly?

Thank you!

timydaley commented 7 years ago

Yeah. I could instead set it to max int initially. Very few people would choose that random seed.

Timothy Daley

Stanford University, Departments of Statistics and Bioengineering.

From: Nathan Boley notifications@github.com Sent: Friday, May 26, 2017 11:07:02 AM To: smithlabcode/preseq Cc: Timothy Patrick Daley; Comment Subject: Re: [smithlabcode/preseq] Allow user to specify seed for RNG (#26)

Hi Tim, thanks for looking at this. One question: If I understand the commit correctly, setting the seed to 0 with the command line option would actually result in a random seed being chosen. I think that this behavior is a bit unexpected -- the original PR got around this by initializing the seed to random and then allowing optparse to override this.

Do I understand correctly?

Thank you!

— You are receiving this because you commented. Reply to this email directly, view it on GitHubhttps://github.com/smithlabcode/preseq/pull/26#issuecomment-304351254, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AHBhcolvYPNtNPQD410tlUx7ur6amk0Hks5r9xTGgaJpZM4J8B_H.

jstjohn commented 7 years ago

What's wrong with setting it to a random seed always, then letting the user override that? Fewer lines of code. On Fri, May 26, 2017 at 2:39 PM Timothy Daley notifications@github.com wrote:

Yeah. I could instead set it to max int initially. Very few people would choose that random seed.

Timothy Daley

Stanford University, Departments of Statistics and Bioengineering.

From: Nathan Boley notifications@github.com Sent: Friday, May 26, 2017 11:07:02 AM To: smithlabcode/preseq Cc: Timothy Patrick Daley; Comment Subject: Re: [smithlabcode/preseq] Allow user to specify seed for RNG (#26)

Hi Tim, thanks for looking at this. One question: If I understand the commit correctly, setting the seed to 0 with the command line option would actually result in a random seed being chosen. I think that this behavior is a bit unexpected -- the original PR got around this by initializing the seed to random and then allowing optparse to override this.

Do I understand correctly?

Thank you!

— You are receiving this because you commented. Reply to this email directly, view it on GitHub< https://github.com/smithlabcode/preseq/pull/26#issuecomment-304351254>, or mute the thread< https://github.com/notifications/unsubscribe-auth/AHBhcolvYPNtNPQD410tlUx7ur6amk0Hks5r9xTGgaJpZM4J8B_H

.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/smithlabcode/preseq/pull/26#issuecomment-304393697, or mute the thread https://github.com/notifications/unsubscribe-auth/AAcBBvRJcDKwflB-w31B659vBFEqxz6aks5r90aLgaJpZM4J8B_H .

timydaley commented 7 years ago

Nothing. I just had this already written, it's already put in everything, and I already tested it. So it was fewer lines for me to write and that point in time.

Timothy Daley

Stanford University, Departments of Statistics and Bioengineering.

From: John St. John notifications@github.com Sent: Friday, May 26, 2017 4:39:55 PM To: smithlabcode/preseq Cc: Timothy Patrick Daley; Comment Subject: Re: [smithlabcode/preseq] Allow user to specify seed for RNG (#26)

What's wrong with setting it to a random seed always, then letting the user override that? Fewer lines of code. On Fri, May 26, 2017 at 2:39 PM Timothy Daley notifications@github.com wrote:

Yeah. I could instead set it to max int initially. Very few people would choose that random seed.

Timothy Daley

Stanford University, Departments of Statistics and Bioengineering.

From: Nathan Boley notifications@github.com Sent: Friday, May 26, 2017 11:07:02 AM To: smithlabcode/preseq Cc: Timothy Patrick Daley; Comment Subject: Re: [smithlabcode/preseq] Allow user to specify seed for RNG (#26)

Hi Tim, thanks for looking at this. One question: If I understand the commit correctly, setting the seed to 0 with the command line option would actually result in a random seed being chosen. I think that this behavior is a bit unexpected -- the original PR got around this by initializing the seed to random and then allowing optparse to override this.

Do I understand correctly?

Thank you!

— You are receiving this because you commented. Reply to this email directly, view it on GitHub< https://github.com/smithlabcode/preseq/pull/26#issuecomment-304351254>, or mute the thread< https://github.com/notifications/unsubscribe-auth/AHBhcolvYPNtNPQD410tlUx7ur6amk0Hks5r9xTGgaJpZM4J8B_H

.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/smithlabcode/preseq/pull/26#issuecomment-304393697, or mute the thread https://github.com/notifications/unsubscribe-auth/AAcBBvRJcDKwflB-w31B659vBFEqxz6aks5r90aLgaJpZM4J8B_H .

— You are receiving this because you commented. Reply to this email directly, view it on GitHubhttps://github.com/smithlabcode/preseq/pull/26#issuecomment-304409334, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AHBhchgtSy0qzDJMVdyFvRVDzARgGKHCks5r92LLgaJpZM4J8B_H.

nboley commented 7 years ago

Thanks for doing this! Do you have any idea when this will be merged into master?

Best, Nathan

andrewdavidsmith commented 2 years ago

If this isn't resolved, please open an issue. It easy to add a desired default initial behavior for setting a seed. I think it's pretty good how it is, and likely not the way it was 5 years ago at the time of this discussion.