Skip to content
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

MAINT: Sync with randomgen changes #2

Merged
merged 3 commits into from Apr 1, 2019

Conversation

bashtage
Copy link

@bashtage bashtage commented Apr 1, 2019

Synchronize with changes in randomgen

This is a very large PR that brings in all of the changes from randomgen.

The most important are:

  1. Remove LegacyGenerator
  2. Add a stand-alone RandomState that will be frozen, following @rkern's suggestion.
  3. Quite a few small bug fixes -- coverage is near 100% now.
  4. SOme refactoring to remove duplicated code, expecially in the basic RNGs.

I'm not sure this is the best way tot get these in. I fyou think it is better to pull from randomgen then please pull these in the best way.

Synchronize with changes in randomgen
@mattip
Copy link
Owner

mattip commented Apr 1, 2019

Absolutely better than me trying to merge it from bashtage/randomgen.

@bashtage
Copy link
Author

bashtage commented Apr 1, 2019

It is failing for some strange reason that seems unrelated to these changes:

https://travis-ci.org/bashtage/numpy/builds/514226868

=================================== FAILURES ===================================
______________________ TestAlmostEqual.test_error_message ______________________
self = <numpy.testing.tests.test_utils.TestAlmostEqual object at 0x7fa7b9975c18>
    def test_error_message(self):
        """Check the message is formatted correctly for the decimal value.
           Also check the message when input includes inf or nan (gh12200)"""
        x = np.array([1.00000000001, 2.00000000002, 3.00003])
        y = np.array([1.00000000002, 2.00000000003, 3.00004])
    
        # Test with a different amount of decimal digits
        with pytest.raises(AssertionError) as exc_info:
            self._assert_func(x, y, decimal=12)
        msgs = str(exc_info.value).split('\n')
        assert_equal(msgs[3], 'Mismatch: 100%')
>       assert_equal(msgs[4], 'Max absolute difference: 1.e-05')
E       AssertionError: 
E       Items are not equal:
E        ACTUAL: 'Max absolute difference: 9.999999999621423e-06'
E        DESIRED: 'Max absolute difference: 1.e-05'

@mattip
Copy link
Owner

mattip commented Apr 1, 2019

Strange. Does that reproduce locally?

@bashtage
Copy link
Author

bashtage commented Apr 1, 2019

No, passes locally.

@bashtage
Copy link
Author

bashtage commented Apr 1, 2019

It does repro locally, on both Windows and Linux. I was only running tests under random, and so I didn't see it.

@bashtage
Copy link
Author

bashtage commented Apr 1, 2019

It looks like the precision is being changed somewhere to that it isn't rounding right. This is a formatted string.
`ACTUAL: 'Max absolute difference: 9.999999999621423e-06'

I also tried rebasing which didn't fix things.

@bashtage
Copy link
Author

bashtage commented Apr 1, 2019

It should be 1.00000e-05

@mattip
Copy link
Owner

mattip commented Apr 1, 2019

Interesting, it is similar to an issue we have on linux32 wheel builds where something is reducing floating point accuracy. A compiler glitch? Some assemble code somewhere changing registers?

Incorporate testing of edge cases into main tests
Rename test files to describe their purpose
@bashtage
Copy link
Author

bashtage commented Apr 1, 2019

It is my fault - I left a np.set_printiontions(precision=20) in a test.

@@ -6,6 +7,7 @@ from randomgen.common cimport *
from randomgen.distributions cimport random_gauss_zig
from randomgen.xoroshiro128 import Xoroshiro128
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from numpy.random.radomgen

@@ -1138,7 +1002,7 @@ cdef class RandomGenerator:
"""
random_integers(low, high=None, size=None)

Random integers of type np.int64 between `low` and `high`, inclusive.
Random integers of type np.int between `low` and `high`, inclusive.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On 32 bit systems this gives int32?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, also on all bitness Windows. This function is deprecated and IMO should be removed before this transition.

Import import locatiosn to reflect numpy paths
@bashtage
Copy link
Author

bashtage commented Apr 1, 2019

This is final from my end. I think it gets it very close, although I know there are still docs and I'm sure other things.

Coverage is pretty good -- should be the same as randomgen:

https://codecov.io/gh/bashtage/randomgen

It is in the low 90s but coverage flags all def lines as not run even when they are, so I suspect that it is probably 98% or so, with only some hard to reach defensive code not covered.

@mattip mattip merged commit ee3593d into mattip:randomgen Apr 1, 2019
@mattip
Copy link
Owner

mattip commented Apr 1, 2019

Thanks. Merging like this is much simpler than hand-importing from randomgen

@bashtage bashtage deleted the randomgen-update branch April 15, 2019 15:49
mattip pushed a commit that referenced this pull request Aug 23, 2019
mattip pushed a commit that referenced this pull request Apr 16, 2021
mattip pushed a commit that referenced this pull request Nov 19, 2021
commit 9c833bed5879d77e625556260690c349de18b433
Author: Thomas Li <47963215+lithomas1@users.noreply.github.com>
Date:   Wed Nov 17 16:21:27 2021 -0800

    Add Windows config to GHA

    update script [wheel build]

    typo [wheel build]

    fix typo? [wheel build]

    fix linux builds? [wheel build]

    typo [wheel build]

    add license and pin to windows 2016

    skip tests [wheel build]

    pin to windows 2019 instead [wheel build]

    try to find out the error on windows [wheel build]

    maybe fix? [wheel build]

    maybe fix? [wheel build]

    fix? [wheel build]

    cleanup [wheel build]

    Add Windows config to GHA

    update script [wheel build]

    typo [wheel build]

    fix typo? [wheel build]

    fix linux builds? [wheel build]

    typo [wheel build]

    add license and pin to windows 2016

    skip tests [wheel build]

    pin to windows 2019 instead [wheel build]

    try to find out the error on windows [wheel build]

    maybe fix? [wheel build]

    maybe fix? [wheel build]

    fix? [wheel build]

    cleanup [wheel build]

    Update LICENSE_win32.txt

    Update LICENSE_win32.txt

    Add Windows config to GHA

    update script [wheel build]

    typo [wheel build]

    fix typo? [wheel build]

    fix linux builds? [wheel build]

    typo [wheel build]

    add license and pin to windows 2016

    skip tests [wheel build]

    pin to windows 2019 instead [wheel build]

    try to find out the error on windows [wheel build]

    maybe fix? [wheel build]

    maybe fix? [wheel build]

    fix? [wheel build]

    cleanup [wheel build]

    Update LICENSE_win32.txt

    Update LICENSE_win32.txt

    Update cibw_test_command.sh

commit 4bd12df
Author: Thomas Li <47963215+lithomas1@users.noreply.github.com>
Date:   Mon Nov 15 17:28:47 2021 -0800

    # This is a combination of 14 commits.
    # This is the 1st commit message:

    Add Windows config to GHA
    # This is the commit message #2:

    update script [wheel build]
    # This is the commit message #3:

    typo [wheel build]
    # This is the commit message #4:

    fix typo? [wheel build]
    # This is the commit message #5:

    fix linux builds? [wheel build]
    # This is the commit message #6:

    typo [wheel build]
    # This is the commit message #7:

    add license and pin to windows 2016
    # This is the commit message #8:

    skip tests [wheel build]
    # This is the commit message #9:

    pin to windows 2019 instead [wheel build]
    # This is the commit message #10:

    try to find out the error on windows [wheel build]
    # This is the commit message #11:

    maybe fix? [wheel build]
    # This is the commit message #12:

    maybe fix? [wheel build]
    # This is the commit message #13:

    fix? [wheel build]
    # This is the commit message #14:

    cleanup [wheel build]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants