New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pcg #1

Merged
merged 32 commits into from Sep 26, 2017

Conversation

Projects
None yet
3 participants
@thewilsonator
Contributor

thewilsonator commented Nov 24, 2016

Have still yet to figure all the permutations of the typedefs.

@thewilsonator

This comment has been minimized.

Show comment
Hide comment
@thewilsonator

thewilsonator Nov 24, 2016

Contributor

CI isn't actually doing anything atm.

Contributor

thewilsonator commented Nov 24, 2016

CI isn't actually doing anything atm.

Show outdated Hide outdated source/random/engine/pcg.d Outdated
@9il

This comment has been minimized.

Show comment
Hide comment
@9il

9il Nov 24, 2016

Member

Does it accept unpredictable seed?

Member

9il commented Nov 24, 2016

Does it accept unpredictable seed?

@thewilsonator

This comment has been minimized.

Show comment
Hide comment
@thewilsonator

thewilsonator Nov 24, 2016

Contributor

Ah I missed that amongst all the template crap.

Contributor

thewilsonator commented Nov 24, 2016

Ah I missed that amongst all the template crap.

@9il

This comment has been minimized.

Show comment
Hide comment
@9il

9il Nov 24, 2016

Member

default constructor should be disabled too

Member

9il commented Nov 24, 2016

default constructor should be disabled too

@9il

needs more docs

Show outdated Hide outdated source/random/engine/pcg.d Outdated
Show outdated Hide outdated source/random/engine/pcg.d Outdated
Show outdated Hide outdated source/random/engine/pcg.d Outdated
Show outdated Hide outdated source/random/engine/pcg.d Outdated
Show outdated Hide outdated source/random/engine/pcg.d Outdated
@9il

This comment has been minimized.

Show comment
Hide comment
@9il

9il Nov 27, 2016

Member

CI fails

Member

9il commented Nov 27, 2016

CI fails

thewilsonator added some commits Nov 27, 2016

@thewilsonator

This comment has been minimized.

Show comment
Hide comment
@thewilsonator

thewilsonator Nov 27, 2016

Contributor

Fixed, but don't merge yet. I still have to wade through a swamp of typdefs, and do the docs.

Contributor

thewilsonator commented Nov 27, 2016

Fixed, but don't merge yet. I still have to wade through a swamp of typdefs, and do the docs.

@9il

This comment has been minimized.

Show comment
Hide comment
@9il

9il Dec 14, 2016

Member

Hi @thewilsonator
Random engine declaration was changed. No udas required, only enum isRandomEngine = true;

Member

9il commented Dec 14, 2016

Hi @thewilsonator
Random engine declaration was changed. No udas required, only enum isRandomEngine = true;

@thewilsonator

This comment has been minimized.

Show comment
Hide comment
@thewilsonator

thewilsonator Dec 14, 2016

Contributor

I think you mean me :P
I'll do the docs probably over the week following christmas, I'm rather busy with my thesis atm.

Contributor

thewilsonator commented Dec 14, 2016

I think you mean me :P
I'll do the docs probably over the week following christmas, I'm rather busy with my thesis atm.

@9il

This comment has been minimized.

Show comment
Hide comment
@9il

9il Dec 14, 2016

Member

I think you mean me :P

Yes, sorry. Your avatars slightly similar to me))

Member

9il commented Dec 14, 2016

I think you mean me :P

Yes, sorry. Your avatars slightly similar to me))

@9il

This comment has been minimized.

Show comment
Hide comment
@9il

9il Jan 2, 2017

Member

@thewilsonator is it ready for review?

Member

9il commented Jan 2, 2017

@thewilsonator is it ready for review?

@thewilsonator

This comment has been minimized.

Show comment
Hide comment
@thewilsonator

thewilsonator Jan 3, 2017

Contributor

I can't remember how much in the way of docs you wanted me to add. So if you're fine with the docs only the unittests need attention. Normally I would do a foreach over an AliasSeq of the aliases at the bottom, but I assume you don't want to depend on Phobos? (Don't forget you can edit the PR).

I don't plan on doing the extended generators as: 1) i don't fully understand their implementation, and 2) I think they are less useful. Someone else that needs them is more than welcome to add them.

Contributor

thewilsonator commented Jan 3, 2017

I can't remember how much in the way of docs you wanted me to add. So if you're fine with the docs only the unittests need attention. Normally I would do a foreach over an AliasSeq of the aliases at the bottom, but I assume you don't want to depend on Phobos? (Don't forget you can edit the PR).

I don't plan on doing the extended generators as: 1) i don't fully understand their implementation, and 2) I think they are less useful. Someone else that needs them is more than welcome to add them.

@9il

This comment has been minimized.

Show comment
Hide comment
@9il

9il Jan 4, 2017

Member

All symbols should be either private or be documented using /++ +/ or /// comments. /+ +/ and // comments are not used by documentation generator.

Member

9il commented Jan 4, 2017

All symbols should be either private or be documented using /++ +/ or /// comments. /+ +/ and // comments are not used by documentation generator.

@9il

This comment has been minimized.

Show comment
Hide comment
@9il

9il Jan 7, 2017

Member

For example pcg8_oneseq_once_insecure should have comment. /// ditto is very useful to automatically duplicate a previous comment

Member

9il commented Jan 7, 2017

For example pcg8_oneseq_once_insecure should have comment. /// ditto is very useful to automatically duplicate a previous comment

@9il

This comment has been minimized.

Show comment
Hide comment
@9il

9il Jan 7, 2017

Member

The PR is almost ready. The last one thing is Phobos like documentation

Member

9il commented Jan 7, 2017

The PR is almost ready. The last one thing is Phobos like documentation

@9il

This comment has been minimized.

Show comment
Hide comment
@9il

9il Jan 7, 2017

Member

Please finish it and I will create new version tag

Member

9il commented Jan 7, 2017

Please finish it and I will create new version tag

thewilsonator added some commits Jan 8, 2017

Show outdated Hide outdated source/random/engine/pcg.d Outdated
@9il

This comment has been minimized.

Show comment
Hide comment
@9il

9il Feb 3, 2017

Member

Hey @thewilsonator, could you please finalise this PR?

Member

9il commented Feb 3, 2017

Hey @thewilsonator, could you please finalise this PR?

@thewilsonator

This comment has been minimized.

Show comment
Hide comment
@thewilsonator

thewilsonator Feb 3, 2017

Contributor

That last commit took waay too long to squash that bug. I'll do the other unit tests later tonight.

Contributor

thewilsonator commented Feb 3, 2017

That last commit took waay too long to squash that bug. I'll do the other unit tests later tonight.

@9il

This comment has been minimized.

Show comment
Hide comment
@9il

9il Mar 7, 2017

Member

@thewilsonator is it finished?

Member

9il commented Mar 7, 2017

@thewilsonator is it finished?

@thewilsonator

This comment has been minimized.

Show comment
Hide comment
@thewilsonator

thewilsonator Mar 7, 2017

Contributor

Sorry about that, I am very busy with uni and will be until the end of this semester.

What is there and unit tested works, matching the output of the equivalent c++ code. However AFAIK there are no published tables for PCG to compare or cite, hence the arbitrary values for the unittest.

Contributor

thewilsonator commented Mar 7, 2017

Sorry about that, I am very busy with uni and will be until the end of this semester.

What is there and unit tested works, matching the output of the equivalent c++ code. However AFAIK there are no published tables for PCG to compare or cite, hence the arbitrary values for the unittest.

@9il

This comment has been minimized.

Show comment
Hide comment
@9il

9il Sep 26, 2017

Member

I will push it and then rework something. Please add updates if required.

Member

9il commented Sep 26, 2017

I will push it and then rework something. Please add updates if required.

@9il 9il merged commit 365b81f into libmir:master Sep 26, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@thewilsonator

This comment has been minimized.

Show comment
Hide comment
@thewilsonator

thewilsonator Sep 26, 2017

Contributor

Sorry I completely for got about this. Let me know if there is anything more I need to do.

Contributor

thewilsonator commented Sep 26, 2017

Sorry I completely for got about this. Let me know if there is anything more I need to do.

@9il

This comment has been minimized.

Show comment
Hide comment
@9il

9il Sep 26, 2017

Member

Please review the docs http://docs.random.dlang.io/latest/mir_random_engine_pcg.html and fix them if required.

Member

9il commented Sep 26, 2017

Please review the docs http://docs.random.dlang.io/latest/mir_random_engine_pcg.html and fix them if required.

@thewilsonator

This comment has been minimized.

Show comment
Hide comment
@thewilsonator

thewilsonator Sep 26, 2017

Contributor

Hmmm, that documentation needs some reformatting. Will fix soon.

Contributor

thewilsonator commented Sep 26, 2017

Hmmm, that documentation needs some reformatting. Will fix soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment