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.

Sign up for GitHub

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

Jump to bottom

Make a new release supporting GSL 2.6 #176

Open
leto opened this issue Mar 21, 2020 · 37 comments
Open

Make a new release supporting GSL 2.6 #176

leto opened this issue Mar 21, 2020 · 37 comments

Comments

@leto
Copy link
Owner

leto commented Mar 21, 2020

Yep.
@leto
Copy link
Owner Author

leto commented May 9, 2020

We are now a lot closer from all the hard work by @hakonhagland and others. What else needs to be done for our next release?

@pdupre0
Copy link

pdupre0 commented Jun 2, 2020

Hello,

I need to have this issue fixed. I just upgrade fedora 30 to fedora 32. and all my application are unavailable because perl-Math-GSL is not compatable with GSL 2.6;
I tried to fix it, but I was not successful.
I would be happy to do more, but I need guidance.
Thanks

@hakonhagland
Copy link
Contributor

Thanks for reporting. Are you using the system perl? (i.e. not perlbrew perl). If you are using perlbrew perl remove the sudo below. Can you try the following:

yum install swig
git clone https://github.com/leto/math--gsl.git
cd math--gsl
perl Build.PL
sudo ./Build installdeps --cpan_client cpanm
./Build
./Build test
sudo ./Build install

@hakonhagland
Copy link
Contributor

@leto What more do we need to do before pushing the current master to CPAN?

@pdupre0
Copy link

pdupre0 commented Jun 3, 2020 via email

@pdupre0
Copy link

pdupre0 commented Jun 3, 2020 via email

@leto
Copy link
Owner Author

leto commented Jun 3, 2020

@hakonhagland do you want to make this release and I will add you as comaintainer on CPAN?

@hakonhagland
Copy link
Contributor

@leto Thanks, I can try. I have not uploaded anything previously to CPAN, so I might need some more help..

@leto
Copy link
Owner Author

leto commented Jun 8, 2020

@hakonhagland is there anything blocking you now? I just merged a bunch of stuff and fixed the version in POD

@hakonhagland
Copy link
Contributor

hakonhagland commented Jun 8, 2020 •
edited

@leto Great! I added to new PRs : #202 and #203. After they are merged I can generate a tar ball of the current master and upload to CPAN? What do you think? My PAUSE user name is HAKONH if you will add me as a comaintainer I can try and upload it..

@leto
Copy link
Owner Author

leto commented Jun 9, 2020

@hakonhagland I added you as co-maintainer and PAUSE said it accepted it. Try uploading a dist and see what happens 😅

@hakonhagland
Copy link
Contributor

@leto, @ambs I tried to upload now, but seems like I am missing permissions for some of the modules (those with maintainer AMBS):

The following report has been written by the PAUSE namespace indexer.
Please contact modules@perl.org if there are any open questions.

  User: HAKONH (Hakon Haegland)
  Distribution file: Math-GSL-0.41.tar.gz
  Number of files: 1347
  *.pm files: 59
  README: Math-GSL-0.41/README.md
  META-File: Math-GSL-0.41/META.json
  META-Parser: Parse::CPAN::Meta 1.4414
  META-driven index: yes
  Timestamp of file: Tue Jun  9 07:22:23 2020 UTC
  Time of this run: Tue Jun  9 07:42:51 2020 UTC

Status of this distro: Permission missing
=========================================


The following packages (grouped by status) have been found in the distro:

Status: Permission missing
==========================

     module : Math::GSL::QRNG::Halton
     version: 0
     in file: lib/Math/GSL/QRNG.pm
     status : Not indexed because permission missing. Current registered
             primary maintainer is AMBS. Hint: you can always find the
             legitimate maintainer(s) on PAUSE under "View Permissions".

     module : Math::GSL::QRNG::Niederreiter2
     version: 0
     in file: lib/Math/GSL/QRNG.pm
     status : Not indexed because permission missing. Current registered
             primary maintainer is AMBS. Hint: you can always find the
             legitimate maintainer(s) on PAUSE under "View Permissions".

     module : Math::GSL::QRNG::ReverseHalton
     version: 0
     in file: lib/Math/GSL/QRNG.pm
     status : Not indexed because permission missing. Current registered
             primary maintainer is AMBS. Hint: you can always find the
             legitimate maintainer(s) on PAUSE under "View Permissions".

     module : Math::GSL::QRNG::Sobol
     version: 0
     in file: lib/Math/GSL/QRNG.pm
     status : Not indexed because permission missing. Current registered
             primary maintainer is AMBS. Hint: you can always find the
             legitimate maintainer(s) on PAUSE under "View Permissions".

@leto
Copy link
Owner Author

leto commented Jun 9, 2020

@hakonhagland this is a weird error from the fact that I think AMBS released Math::GSL versions that contained those submodules or some weird bug in PAUSE. If we can get AMBS to give you co-maint, it may work

@leto
Copy link
Owner Author

leto commented Jun 9, 2020 •
edited

hey @ambs can you help us out and give HAKONH co-maint on Math::GSL? He has done epic work. I gave him co-maint but it seems you are the only maintainer of some submodules and you also need to do it

@hakonhagland
Copy link
Contributor

@leto Seem like it worked despite the warnings about missing permissions since today I got the a CPAN Testers Daily Summary Report emailed. As you can see there are two test failures. How can we fix them?

@leto
Copy link
Owner Author

leto commented Jun 11, 2020

@hakonhagland welcome to my pain 👼

@leto
Copy link
Owner Author

leto commented Jun 11, 2020

@hakonhagland if you click on the FAIL link for both, and look at the output, you will see they are different errors. One is from GSL.t and the other is SF.t, both precision issues

Precision-related bugs are the bane of the Math::GSL test suite. We discover many precision bugs in some OS's and compiler combinations, and it's a messy situation

@leto
Copy link
Owner Author

leto commented Jun 11, 2020

@hakonhagland one failure seems FreeBSD related (maybe their default compiler) and the other is probably related to Perl being built with quadmath. Have fun!

@leto
Copy link
Owner Author

leto commented Jun 11, 2020

If you are not scared yet, it's likely that the Math::GSL test suite has some TODO tests that are actually bugs in Perl core or related to some weird Perl non-default compilation setting on some weird OS only on some compiler versions 😈

@leto
Copy link
Owner Author

leto commented Jun 11, 2020

@hakonhagland you can TODO those tests on just those platforms, is one "middle path" way to go. To get a release in the hands of users sooner rather than later. I support that

@hakonhagland
Copy link
Contributor

hakonhagland commented Jun 11, 2020 •
edited

@leto Interesting. Now, I am looking at the first failed test for perl 5.20.1 on FreeBSD, it says:

t/SF.t           (Wstat: 0 Tests: 1074 Failed: 1)
  Failed test:  22

It I try run the SF.t test by it self:

$ prove -lv  t/SF.t
[...]
ok 22 - gsl_sf_fermi_dirac_m1_e(10.0, $r) ?= 0.999954602131298, # TODO loss of precision on Perls with DUSELONGDOUBLE
# res= +-0, eps=2.21777808290173e-17
# 
[...]

So test #22 in SF.t seem to be this one TEST_RT66882:

sub TEST_RT66882 : Tests {
    # This test was losing a small amount of precision with
    # perls that have DUSELONGDOUBLE, so give it some leeway
    # Where is the loss of precision coming from? Is it a bug in Perl, GSL or Math::GSL ?
    # A beer if you figure it out.
    # why doesn't $TODO work here?
    local $TODO = "loss of precision on Perls with DUSELONGDOUBLE";

    # additional diagnostics
    local %ENV; $ENV{DEBUG} = 1;
    my $results = {
        'gsl_sf_fermi_dirac_m1_e(10.0, $r)' => 0.9999546021312975656,
    };
    verify_results($results, 'Math::GSL::SF', 1e-16);
}

The RT ticket: Fails on perls compiled with -Duselongdouble. The strange thing is that it is already a TODO test and as the comment says "why doesn't $TODO work here?"

@leto
Copy link
Owner Author

leto commented Jun 11, 2020

@hakonhagland welcome to the madness. That comment is like 10 years old 😅

@leto
Copy link
Owner Author

leto commented Jun 11, 2020

@hakonhagland I am OK with completely disabling those 2 tests if we have to. They are longstanding issues and not related to your recent code

@leto
Copy link
Owner Author

leto commented Jun 11, 2020

@hakonhagland my guess is that $TODO is not set in the correct lexical scope, it should probably be passed into the verify_results function and then it might work

@hakonhagland
Copy link
Contributor

@leto Ok.. I will have a look at it. But now I am starting to having some suspicion that the test numbers from prove -lv t/SF.t compared to running ./Build test are not the same.. I am looking into it.. The report says:

Failed test:  22

How can I determine which test is test # 22 ?

@hakonhagland
Copy link
Contributor

hakonhagland commented Jun 11, 2020 •
edited

@leto I think the test # 22 is correct.. But if you look at the other failed test for perl 5.28.3 on GNU/Linux, it says:

t/GSL.t          (Wstat: 256 Tests: 21 Failed: 1)
  Failed test:  4

And from the output above that line:

#   Failed test 'is_similar(0.10005,0.1000501, 1e-7) ?= 1,
# res= +-1, eps=1e-08'
#   at t/GSL.t line 50.
#   (in Math::GSL::GSL::Test->TEST_STUFF)

so it seems like it is test 'is_similar(0.10005,0.1000501, 1e-7) that fails. Notice that this test is defined as, see t/GSL.t

sub TEST_STUFF : Tests {
    {
        my $results = {
                    q{is_similar(undef, [1,2,3]) }                        => [ 0 ],
                    q{is_similar(0.10005,0.1000501, 1e-5)}                => [ 1 ],
                    q{is_similar(0.10005,0.1000501, 1e-7)}                => [ 0 ],
                    q{is_similar([1,2,3    ], [1,2,3.001])}               => [ 0 ],
                    q{is_similar([1,2,3.001], [1,2,3.001])}               => [ 1 ],
                    q{is_similar([1,2,3.001], [1,2,3.001],1e-2)}          => [ 1 ],
                    q{is_similar([1,2,3.0010001], [1,2,3.0010002], 1e-5)} => [ 1 ],
                    q{is_similar([1,2,3.0010001], [1,2,3.0010002] )}      => [ 0 ],
                    q{is_similar_relative( 1e8, 1e8 + 1, 1e-7) }          => [ 1 ],
                    q{is_similar_relative( 1e8, 1e8 + 1e3, 1e-7) }        => [ 0 ],
                    q{is_status_ok($GSL_SUCCESS)}                         => [ 1 ],
                    q{is_status_ok($GSL_EDOM)}                            => [ 0 ],
        };

        verify($results, 'Math::GSL');
    }
    # [...]

Since the test is one entry in the hash $results = { ... } see code above, and the order of the keys in a hash is not well defined in Perl, I think the number of the failed test will not always be # 4..

@hakonhagland
Copy link
Contributor

hakonhagland commented Jun 11, 2020 •
edited

@leto This other failed test for perl 5.28.3 on GNU/Linux is really strange though. It seems to me the test simply does

Math::GSL::is_similar(0.10005,0.1000501, 1e-7)

which checks if abs(0.10005 - 0.1000501) < 1e-7 . However, if I on my machine run

$ perl -E '$_ = abs(0.10005 - 0.1000501); say; say "Test will fail" if $_>1e-7'
1.00000000002876e-07
Test will fail

So I am not sure what is going on here..

@leto
Copy link
Owner Author

leto commented Jun 11, 2020

@hakonhagland you can inspect the full TAP output which is numbered. You probably want these bash aliases:

### testing aliases
function bt () {
    perl Build.PL && ./Build test --verbose 1 --test_files $@
}
function t () {
    prove -blrv $@ 
}

@hakonhagland
Copy link
Contributor

Could this line in lib/Math/GSL/Test.pm cause undefined behavior?

$delta > $eps ? diag qq{\t\t\$x=$x\n\t\t\$y=$y\n\t\tdelta=$delta\n} && return 0 : return 1;

the && operator depends on diag to return non-zero value right? As I understand the code, the meaning is to execute like this:

if ($delta > $eps) {
   diag qq{\t\t\$x=$x\n\t\t\$y=$y\n\t\tdelta=$delta\n};
   return 0;
}
else {
    return 1;
}

@leto
Copy link
Owner Author

leto commented Jun 12, 2020

@hakonhagland that code looks like my shitty code from over 10+ years ago, feel free to refactor and improve it 😄

@pdupre0
Copy link

pdupre0 commented Jun 12, 2020 via email

@hakonhagland
Copy link
Contributor

hakonhagland commented Jun 12, 2020 •
edited

@pdupre0 Thanks for the comment. I think I am starting to understand what is going on: on line 38 we have:

q{is_similar(0.10005,0.1000501, 1e-7)}                => [ 0 ],

This will compare 0.10005 and 0.1000501 by calculating (se line 96 in Test.pm):

my $delta = abs($x-$y);

where $x = 0.10005 and $y = 0.1000501. The exact value of $delta should be 1e-7, but the creator of the test assumed (?) that the arithmetic was not exact. so $delta was assumed to be somewhat greater than 1e-7. Hence, on the next, line 97, we usually return a zero:

$delta > $eps ? diag qq{\t\t\$x=$x\n\t\t\$y=$y\n\t\tdelta=$delta\n} && return 0 : return 1;

since $eps == 1e-7. However on systems with quadmath (the systems where the test fails) $delta can become exactly equal to 1e-7 and so line 97 will return a 1 instead of the expected 0, and the test fails. Is this correct?

@pdupre0
Copy link

pdupre0 commented Jun 12, 2020 via email

@leto
Copy link
Owner Author

leto commented Jun 12, 2020

@hakonhagland the test is correct, the bug lies in Perl, GSL or Math::GSL, as per the comment

@leto
Copy link
Owner Author

leto commented Jun 12, 2020

I could give a 20 hour talk on writing tests dealing with floating point, but nobody would pay me 😅

@hakonhagland just disable these tests completely or make them TODO on the platforms that fail, is my suggestion. If you do fix those 12+ year old bugs, you definitely earn a 🍺 from me

@hakonhagland
Copy link
Contributor

@leto Ok you are probably right.. still I do not get it though, look at this code at line 38:

q{is_similar(0.10005,0.1000501, 1e-7)}                => [ 0 ],

the 0 in the brackets [0] means check if $delta is greater than not less than, right? If we want to check if $delta is less than 1e-7 we should supply a [1] not a [0] right, or am I missing something?

@hakonhagland hakonhagland mentioned this issue Jun 12, 2020
Open
hakonhagland added a commit to hakonhagland/math--gsl that referenced this issue Jun 12, 2020
@hakonhagland
Fix failed test GNU/Linux with quadmath.
9ece2d7 
This test fails for perl 5.28.3 on GNU/Linux with quadmath. The reason
is still unclear, see issue leto#176 for more information. We temporarily
disable the test to make the test suite pass.
@hakonhagland hakonhagland mentioned this issue Jun 12, 2020
Merged
@mohawk2
Copy link

mohawk2 commented Mar 20, 2021

Exact floating-point comparison is a well-known problem, so I feel your pain here! I'm just looking at expanding PDL's support for GSL, so looking at this excellent project for ideas to "be inspired by, verbatim" (steal).

Given that 0.41 supports 2.6, should this issue be closed and/or a new one opened for the precision issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@leto @hakonhagland @mohawk2 @pdupre0