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

test random failed #2542

Closed
certik opened this issue Feb 17, 2024 · 5 comments
Closed

test random failed #2542

certik opened this issue Feb 17, 2024 · 5 comments
Assignees

Comments

@certik
Copy link
Contributor

certik commented Feb 17, 2024

167/325 Test #174: test_random_02 ...................***Failed    0.12 sec
0.7547386333122814 0.7539588431983508
Traceback (most recent call last):
  File "/home/runner/work/lpython/lpython/integration_tests/test_random_02.py", line 11, in <module>
    test_seed()
  File "/home/runner/work/lpython/lpython/integration_tests/test_random_02.py", line 9, in test_seed
    assert abs(t1 - t2) > 1e-3
           ^^^^^^^^^^^^^^^^^^^
AssertionError

https://github.com/lcompilers/lpython/actions/runs/7939986580/job/21680720142?pr=2540

@certik
Copy link
Contributor Author

certik commented Feb 17, 2024

The test is flaky:

assert abs(t1 - t2) > 1e-3
, one can't test random numbers like that.

I thought we already tackled this issue before with @Shaikh-Ubaid in LFortran. One must test random numbers using tests designed for random numbers: they pass with very high probability (=every time in practice). The above test is low probability, that one can even trigger a failure at a CI. So that's a bad test. We need to improve it.

@Shaikh-Ubaid
Copy link
Collaborator

A good test exists in LFortran. The test in LPython was more for completeness or as measure of extra safety.

Since, it already has tests in LFortran, do we still need to add a similar test for LPython?

@certik
Copy link
Contributor Author

certik commented Feb 17, 2024

Just test it in some way that is robust in LPython as well, to ensure that everything works.

@syheliel
Copy link
Contributor

I have added a PR for it. The new testcase's aim is to test whther the distribution is uniform. I think it's a more valuable test compared to original version

@certik
Copy link
Contributor Author

certik commented Feb 18, 2024

I think this is fixed now.

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

No branches or pull requests

3 participants