Lets start with - I found this issue because I was stupid.
Many cmdline tools accept short and long form arguments.
So, when I read the seqtk sample help, I assumed we could provide either -sXXX or -seed=XXX for the seed:
Usage: seqtk sample [-2] [-s seed=11] <in.fa> <frac>|<number>
Options: -s INT RNG seed [11]
-2 2-pass mode: twice as slow but with much reduced memory
When providing -seed=XXX, seqtk sample doesn't fail, it silently uses 0 as the seed.
This happens because:
The number of arguments provided passes the getopt() call here.
The string check passes here.
Then, atol is passed a null string and initializes kr_srand() with a seed of 0.
(I was expecting the default value, 11, but it behaves like -s0 instead)
The same thing happens if we pass -se as the seed.
(Tested b/c I thought it was parsing e as its ascii value, 101. This is not the case.)
-seed XXX does fail, which is nice.
While it would be cool to parse long-form args, I don't think that's worth the effort.
I think clarifying the documentation is more straightforward:
Change this line from [-s seed=11] to just [-s]
Change this line from RNG seed [11]\n to RNG seed [default=11]\n
That would make the accepted argument clear while also denoting the default seed.
~Jared
Lets start with - I found this issue because I was stupid.
Many cmdline tools accept short and long form arguments.
So, when I read the
seqtk samplehelp, I assumed we could provide either-sXXXor-seed=XXXfor the seed:When providing
-seed=XXX,seqtk sampledoesn't fail, it silently uses0as the seed.This happens because:
The number of arguments provided passes the
getopt()call here.The string check passes here.
Then,
atolis passed anullstring and initializeskr_srand()with a seed of 0.(I was expecting the default value, 11, but it behaves like
-s0instead)The same thing happens if we pass
-seas the seed.(Tested b/c I thought it was parsing
eas its ascii value, 101. This is not the case.)-seed XXXdoes fail, which is nice.While it would be cool to parse long-form args, I don't think that's worth the effort.
I think clarifying the documentation is more straightforward:
Change this line from
[-s seed=11]to just[-s]Change this line from
RNG seed [11]\ntoRNG seed [default=11]\nThat would make the accepted argument clear while also denoting the default seed.
~Jared