Skip to content

BUG: amos/amos.h: fix scope of l in seri #92

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

Merged
fbourgey merged 9 commits into from
Jun 9, 2026
Merged

BUG: amos/amos.h: fix scope of l in seri #92

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
e6c80f2
BUG: amos/amos.h: fix scope of l in seri
JaRoSchm Feb 3, 2026
8ef7b48
Merge branch 'scipy:main' into amos_seri_fix
JaRoSchm May 6, 2026
71da72e
Merge branch 'scipy:main' into amos_seri_fix
JaRoSchm May 11, 2026
1beee0b
TST: add test for gh-92
JaRoSchm May 11, 2026
ce75f4b
Merge branch 'main' into amos_seri_fix
JaRoSchm Jun 5, 2026
ba780bd
TST: add test for https://github.com/scipy/xsf/pull/92
JaRoSchm Jun 8, 2026
a973ddd
revert fix
JaRoSchm Jun 8, 2026
8a3cb53
Update tests/xsf_tests/test_amos.cpp
JaRoSchm Jun 9, 2026
9f9fac1
revert 'revert fix'
JaRoSchm Jun 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions include/xsf/amos/amos.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4476,8 +4476,8 @@ namespace amos {
//
s1 = w[0];
s2 = w[1];
l = 3;
for (int l = 3; l < (nn + 1); l++) {
int l;
for (l = 3; l < (nn + 1); l++) {
Comment on lines +4479 to +4480

@fbourgey fbourgey Jun 7, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, this seems to fix the translation error.

ck = s2;
s2 = s1 + (ak + fnu) * rz * s2;
s1 = ck;
Expand Down
30 changes: 30 additions & 0 deletions tests/xsf_tests/test_amos.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,36 @@ TEST_CASE("amos besi vectorized", "[amos][xsf_tests]") {
}
}

TEST_CASE("amos besh vectorized", "[amos][xsf_tests]") {

@fbourgey fbourgey Jun 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test seems useful but should we not add a sentinel regression test as you did #158? It passes on main.

@JaRoSchm JaRoSchm Jun 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for checking! Seems like I have mixed up what this PR actually fixes with some other PR. I have now added a test similar to #153 here.

// tests the functionality of amos to return multiple consecutive orders for besh
// by comparing to the versions returning only a single order
using std::complex;

using test_case = std::tuple<complex<double>, double, int, int, int, double>;
auto [z, fnu, kode, m, n, rtol] = GENERATE(
test_case{complex{14.0, -3.0}, 1.0, 1, 1, 260, 4e-13} // gh-92
);

std::vector<complex<double>> cy(n);
int ierr = 0;
int nz;

nz = xsf::amos::besh(z, fnu, kode, m, n, cy.data(), &ierr);

REQUIRE(ierr == 0);

complex<double> ref;

for (int i = 0; i < n; ++i) {
nz = xsf::amos::besh(z, fnu + i, kode, m, 1, &ref, &ierr);
REQUIRE(ierr == 0);

const auto rel_error = xsf::extended_relative_error(cy[i], ref);
CAPTURE(i, cy[i], ref, rel_error);
REQUIRE(rel_error <= rtol);
}
}

TEST_CASE("amos asyi buffer overflow gh-158", "[amos][xsf_tests]") {
using std::complex;

Expand Down
Loading