-
Notifications
You must be signed in to change notification settings - Fork 3
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
1D GRPIC #59
base: 1.2.0rc
Are you sure you want to change the base?
1D GRPIC #59
Conversation
@StaticObserver nice! i changed the merge branch to 1.1.0rc. we try to never directly merge to master. |
@StaticObserver i rebased the branch so it's now up-to-date with 1.1.0rc. Only changes relevant to the new metric are visible now. |
|
The tests all passed on HIP device and failed on Cuda device. I carefully checked the expressions of f0 and find no errors, I simply changed the accuracy requirement from 10 to 30, and they all should pass now. |
Sorry, I just found where to change the branch to merge |
you mean in GH actions? it actually doesn't run the AMD, which is why the circle is orange. the nvidia ones are ran automatically on my computer whenever i log in (github doesn't provide GPUs for free). i haven't set up the HIP container for gh actions yet, that one will have to run on my laptop. if you click details on the failed test and scroll down, you should see where it fails. in this case, for instance, it fails at test 11 (double precision):
from the looks of it, it seems the issue is in floating point comparison. unfortunately, this is a very sensitive operation, and often depends on the architecture/compiler etc. so in these tests, i usually try to compare numbers with a large-enough margin of error. i.e., |
Single precision tests also fail at test 11:
and here the difference is more prominent, but again likely due to truncation error. |
I tried my best to reduce the truncation error. |
Inline auto eta2r(const real_t& eta) const -> real_t{
real_t diff = TWO * math::sqrt(ONE - SQR(a));
real_t exp_m1 = std::expm1(eta * diff);
return rh_m - diff / exp_m1;
} you cannot use so |
I see, but they worked well, i guess it was just not efficient? |
@StaticObserver all your pushes will be published here automatically. no need for additional PR. |
I see, thanks for reminding. |
I finished tests of new metric, they all passed