-
Notifications
You must be signed in to change notification settings - Fork 45
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
Prepare release 3.4.0. #49
Comments
So far, it looks like RC1 is doing fine on Debian. See the build logs for all supported architectures here. |
Our calculation of epsilon fails in |
A quick and dirty fix might be to define NFFT_EPSILON=2^-105 if the calculated value is too small (e.g. <10^-40). |
I did use the double double implementation years ago on an Apple PowerBook. It's a bit fiddly. The workaround sounds ok to me. Longer term, is there a more robust way to calculate an epsilon in this case? It seems that our epsilon is calculated correctly, but it looses some of the properties we assume.
… Am 22.09.2017 um 15:21 schrieb Michael Quellmalz ***@***.***>:
A quick and dirty fix might be to define NFFT_EPSILON=2^-105 if the calculated value is too small (e.g. <10^-40).
—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
That's right. |
Thinking about it, we could also rely on LDBL_MANT_DIG to limit the number of iterations in our calculation.
When I implemented it, I was still thinking that LDBL_EPSILON was just rubbish on PowerPC, but it's actually defined correctly. But the "epsilon" we want is here better defined as 2^(-LDBL_MANT_DIG+1).
… Am 22.09.2017 um 15:21 schrieb Michael Quellmalz ***@***.***>:
A quick and dirty fix might be to define NFFT_EPSILON=2^-105 if the calculated value is too small (e.g. <10^-40).
—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
Taking the length of the mantissa should be the better solution. |
Ok then let's make the change and try again.
… Am 22.09.2017 um 17:19 schrieb Michael Quellmalz ***@***.***>:
Taking the length of the mantissa should be the better solution.
—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
We have created the tag @ghisvail Could you run the tests again for rc2 with enabled @jenskeiner I still use the computation of epsilon you implemented in case it cannot get MAND_DIG from |
Quick question: is there something like a |
The tests are tied to the "check" target. Do you mean to disable the tests even when "make check" is called explicitly?
… Am 28.09.2017 um 12:57 schrieb Ghislain Antony Vaillant ***@***.***>:
Quick question: is there something like a --disable-tests or --disable-check option which allows building the library without its test suite?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
Agreed, we should remove the epsilon calculation. Our assumptions about eps are anyway invalid for double double, so the fallback is useless here.
… Am 26.09.2017 um 09:49 schrieb Michael Quellmalz ***@***.***>:
We have created the tag 3.4.0-rc2.
@ghisvail Could you run the tests again for rc2 with enabled make check in all architectures?
@jenskeiner I still use the computation of epsilon you implemented in case it cannot get MAND_DIG from float.h. Maybe we should delete this and rely only on MANT_DIG.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
@jenskeiner Nevermind. I am testing a build for rc2 now. |
It's a fail for |
The unthreaded library did fairly well, it only missed one threshold a little (< 10%), so the threshold should be adjusted. But something is wrong in |
I would advise against disabling the checks, especially since we have a relative error of up to 1 in nfft_adjoint on ppc64el long double with OpenMP. @ghisvail Is it possible to disable the long double version just for ppc64el? Otherwise, I suggest to disable the long double version for all architectures supported by Debian for now until we have solved the issue in one of the next releases. By the way, I applied for access to the OpenPower Cloud by Unicamp and I am going to try to reproduce the errors there. In the meantime, I propose to publish the release 3.4.0 |
I agree, let's accept the errors for now. Anything else we need to take care of before the release? If not, I'll tag the final version soonish.
… Am 16.10.2017 um 14:32 schrieb Toni Volkmer ***@***.***>:
I would advise against disabling the checks, especially since we have a relative error of up to 1 in nfft_adjoint on ppc64el long double with OpenMP.
@ghisvail Is it possible to disable the long double version just for ppc64el? Otherwise, I suggest to disable the long double version for all architectures supported by Debian for now until we have solved the issue in one of the next releases.
By the way, I applied for access to the OpenPower Cloud by Unicamp and I am going to try to reproduce the errors there.
In the meantime, I propose to publish the release 3.4.0
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
I also agree. I think we can release 3.4.0. |
Fine with me. |
I have access to the OpenPower Cloud now. Until now, I have not been able to reproduce the large errors with long double precision and OpenMP, which we observed on debomatic-ppc64el.debian.net Besides, there is still a failure for long double precision and non-OpenMP in adjoint_direct on ppc64el due to a minor violation of a threshold which I'm going to fix. |
There is a new release candidate nfft-3.4.0-rc3. @ghisvail: If possible, would you please run the Debian builds again for this and especially on debomatic-ppc64el.debian.net? |
@tvolkmer |
Ok, I'll cut the release now anyway. Thanks everybody for your contributions. |
So the plan is still to disable testing for long-double for powerpc architectures I suppose? |
@ghisvail Thank you for running the tests. In some test cases (multithreaded OpenMP), there is a relative error of 1, which clearly means that something is totally wrong there. However, I am not able to reproduce these problems on the OpenPower Cloud. |
This task is to prepare release 3.4.0.
The text was updated successfully, but these errors were encountered: