Lagged Fibonacci Generator #727
Conversation
|
I will be away from August 6 to August 20. Apologies for not being able to answer comments during that time. |
| Type[longLag] x; | ||
| size_t i = 0; | ||
|
|
||
| void ThrowSeedException() |
|
ping? |
|
Sorry, saw the notice only now. Let's get back to this after Aug 20. Enjoy vacations! |
|
due for review next week |
|
Thanks a lot for the re-read. In my defense, I tried to stay close to the boost code, but I think your comments are pertinent, and the code should be more D-Like. I have nothing to add to any of the comments, except the "ref?" one. Will provide a new version in 2 days. |
|
Ok, I did all the above fixes, plus a few internal tweaks. Also added the unittests I had forgotten to add. Added complexity ratings, and merged a few docs with ditto's. |
| Tells if the generator has been seeded/initialized | ||
| */ | ||
| @property @safe nothrow const | ||
| bool seeded() |
There was a problem hiding this comment.
Is this a good name? Maybe "isSeeded" instead?
Property?
There was a problem hiding this comment.
seeded is fine as long as it's a property
| } | ||
| } | ||
| } | ||
| Payload* payload; |
There was a problem hiding this comment.
Isn't it simpler to just let the GC handle it? I don't think there is any reason to force RAII on the allocated space here.
There was a problem hiding this comment.
fine (this actually works great with the suggestion to make this a class)
|
Getting there. Nice work! |
|
Thank you again for the re-read. I did not have time to work on it this week, but I needed your feedback on the constructor/payload issue before continuing anyways. Regarding everything else, I think one more week of work, and everything should be final. D is still a learning process for me, so I don't want to rush this. |
|
Just do what the Mersenne Twister does (i.e. ctor with seed and also seed method). We can always add the opCall trick if we decide it's worth it. For now let's aim for conservative uniformity. Thanks! |
|
Fixed what needed to be fixed. Made everything as conservative as and straight forward as possible. I kept the reference semantics. It is kind of discussed in this thread: Long story short: It is much more convenient for the type itself to provide reference semantics, then for the developers to do it themselves. As a reminder, LaggedFibonacci44497 is 300K in size (!) Anyways, I don't think there is much else to say here on my end. I was still curious about nested structs and have a hidden "outer" pointer: Could we have your input in this discussion? Or here and I'll relay. |
Ported from boost.
| * numbers in the range [0 .. 2^^bits). Real Type will generate | ||
| * floating points in the range [0 .. 1), with a step of 1/2^^bits. | ||
| * | ||
| * The engine uses reference semantics, and models a ForwardRange. |
There was a problem hiding this comment.
s/ForwardRange/$(D ForwardRange)/
There was a problem hiding this comment.
The fact that it uses reference semantics must be emphasize and discussed with examples. Documentation should also mention that this semantics is different from all other types in this module.
|
Closed for development |
Ported from boost:
http://www.boost.org/doc/libs/1_50_0/boost/random/linear_congruential.hpp
While boost provided two different classes for UInt/Real types, this implementation merges both into the same class.
My doubts on the implementation are:
*Aliases: Boost uses doubles as the defaults for the engine, and doesn't provide any aliases for the unsigned types. Should we follow the same scheme? Provided alliases for both?
*Default Alias: Not sure which config to use for the "LaggedFibonacci", so I chose LaggedFibonacci607, but documented it as "implementation chosen".
*Reference type & unseeded engine: The generator is a reference type forward range. In thoery, it can't be used until it has been seeded. I have put in asserts about this. Maybe it should be enforece instead?
*Boost provides a "generate" function, which is used to fill a range of integers. I did not write these, as they can be replaced by simply chosing the correct engine, an manually calling std.algo.fill.
Open to all suggestions/criticism, even the "minor" ones, such as documentation/licensing.