BUG: amos.h: fix translation errors in asyi, unk1, and besi#158
Conversation
| } | ||
| ck = std::exp(cz); | ||
| for (int i = 0; i < (nn + 1); i++) { | ||
| for (int i = 0; i < nn; i++) { |
There was a problem hiding this comment.
| } | ||
| nz = n; | ||
| for (i = 0; i < (n + 1); i++) { | ||
| for (i = 0; i < n; i++) { |
There was a problem hiding this comment.
| aphi = std::abs(phid); | ||
| aarg = std::abs(argd); | ||
| rs1 += std::log(aphi) - 0.25 * std::log(aarg) - aic; | ||
| rs1 += std::log(aphi); |
There was a problem hiding this comment.
JaRoSchm
changed the title
|
I added a fix for another translation bug in |
| return 0; | ||
| } | ||
| if (xx > 0.0) { | ||
| if (xx >= 0.0) { |
There was a problem hiding this comment.
There was a problem hiding this comment.
LGTM, thanks!
I potentially see one more
From https://github.com/scipy/scipy/blob/4edfcaa3ce8a387450b6efce968572def71be089/scipy/special/amos/zunk2.f#L60-L62, it seems https://github.com/scipy/xsf/blob/main/include/xsf/amos/amos.h#L5879 should be changed to
double bry[3] = {1e3 * d1mach[0] / tol, tol / (1e3 * d1mach[0]), d1mach[1]};@JaRoSchm can you confirm?
|
@fbourgey Thank you for the review!
Yeah, your are right. But this fix is already included in #132. :) I notice that splitting the fixes into different PRs might have been a bit confusing. |
| } | ||
| } | ||
|
|
||
| TEST_CASE("amos besi vectorized", "[amos][xsf_tests]") { |
There was a problem hiding this comment.
Can you please add a small regression test for asyi too? A regression test forunk1 and unk2 would be nice too but they look harder to hit.
There was a problem hiding this comment.
Done. Your intuition was right, I did not find a way to hit the fixes in unk1 and unk2.
Great, thanks. Let's get that one in first. |
|
Thanks @JaRoSchm! |
|
Thank you for the review! |
2 participants
Reference issue
None
What does this implement/fix?
After the number of translation errors I found by hand, I decided to try an LLM for comparing the Fortran and C versions of amos. These are the translation errors unveiled this way. The disadvantage is, that I cannot add I test as it's hard to predict how to hit these branches.
Additional information
None
AI Generation Disclosure
Used Gemini 3.5 Flash to compare
amos.fwithamos.h. All suggestions were checked by hand (there was one additional false positive). Using a more powerful (= more expansive) model might also be rewarding.