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.

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

clean up non-standard C++ elements in calculations #17

Closed
sybenzvi opened this issue Aug 16, 2024 · 3 comments
Closed

clean up non-standard C++ elements in calculations #17

sybenzvi opened this issue Aug 16, 2024 · 3 comments
Assignees
Labels
bug Something isn't working

Comments

@sybenzvi
Copy link
Contributor

It looks like there are some non-standard C++ calls in the code that will cause problems for some compilers. Two examples identified by @sanyaarora:

  1. The code assumes M_PI is defined in the math header but this is a non-standard math extension.
  2. In src/BEMEWS/_ext/mstl/math2/analysis/runge kutta.cpp the compiler on her machine is choking on the line static const double b0[]={}; with the error that she cannot allocate an empty array. I also noticed that this allocation seems to implicitly assume the static arrays b0, ..., b5 allocate a contiguous block in memory, which may or may not be respected by all compilers.

There may be other issues that crop up. We'll need people to try installing on windows and linux to get this fully tested and fixed.

@sybenzvi sybenzvi added the bug Something isn't working label Aug 16, 2024
@sybenzvi
Copy link
Contributor Author

What's happening in runge kutta.cpp is the code is defining a ragged table for one of the Runge-Kutta coefficients and the math requires the table index to be unit-offset instead of zero-offset, so the first array in the table is empty. In clang, this compiles fine,

static const double b0[]={};

but I guess this is not standard so it can fail in other compilers. This seems to work:

static const double b0[1]={};

@jpkneller
Copy link
Collaborator

You're right that M_PI is not standard. Since C++20 pi is now standard in the header but many people will not have a compiler which implements that standard. I usually use C++17. Anyway, one fix is to have the precompiler test whether M_PI is defined and if not, define it. I made the change.

The static const double b0[1]={}; fix will work. The contents of b0 are never used for a Runge Kutta method using the Cash-Karp parameters. I made the change.

@jpkneller
Copy link
Collaborator

Code fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants