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

Add a new tester that utilizes tlfloat #574

Open
shibatch opened this issue Sep 16, 2024 · 8 comments
Open

Add a new tester that utilizes tlfloat #574

shibatch opened this issue Sep 16, 2024 · 8 comments

Comments

@shibatch
Copy link
Owner

Why a new feature? Is your feature request related to a problem? Please describe it.

  • The new tester will work without a pipe connecting two processes. Introducing this tester will resolve the posix dependency problem.

  • The new tester can be used in environments where MPFR is not available. It will be much easier to do testing on Windows.

  • I expect the new tester to work faster than the current tester/iut combo.

What feature would you like?

  • The tester compares the output of SLEEF to the output of TLFloat for a predetermined set of inputs.
  • By comparing the outputs to the higher precision output of TLFloat, the tester verifies that the error in SLEEF's outputs is within a predetermined range.

How would you like this feature to be implemented?

  • As described in Incorporating tlfloat as a submodule #568, I will incorporate TLFloat as a submodule of SLEEF.

  • The structure will be like a combination of the existing IUT and tester, and will be built with different settings for each vector extension.

How portable is the feature across architectures and platforms?

  • The purpose of introducing this tester is to address the current portability issues associated with MPFR and pipe.

  • TLFloat has fewer portability issues than MPFR and can be used in a wider range of environments.

  • However, it will require a C++20 compiler and cannot be compiled with clang 16 or earlier due to compiler bugs.

Describe alternative approaches you have considered

  • The current tester/iut will not be deleted.

  • At the time of build, the user can select either the current tester/iut or the new tester to be introduced.

  • I will allow the user to choose whether to build the current tester/iut and the new tester by cmake options.

Additional context

  • The addition of the new tester will not constitute a major code change.

  • However, the addition of C++ code and the introduction of submodules may affect the user's build environment and future development.

  • The default setting is to test with the existing tester/iut, as is the case at present. Only when new cmake options are specified, the new tester will be built and used for testing instead of the current tester/iut.

@blapie
Copy link
Collaborator

blapie commented Sep 30, 2024

Hello, Sorry for the delay in replying.

Here are a few general comments:

  1. This generally seems like an improvement and the integration of the new tester should be fairly smooth.

  2. As mentioned in the previous issue, the main worry is the addition of code, which means more maintenance work. However, because it solves a portability issue (in particular the posix-one) then I think it is worth the maintenance cost.

  3. I believe the project can greatly benefit from C++ in the long term (e.g. leverage templatisation to reduce the need for macros), in particular when it comes to tests and benchmarks. By the way we are currently reworking the benchmark suite using C++.

  4. I think the existing testers could use a bit of documentation (inline and on website) to facilitate maintenance.

Some answers/reactions to your issue:

TLFloat has fewer portability issues than MPFR and can be used in a wider range of environments.

I still wonder what sort of environment does not have access to MPFR? This did not occur to me that this could be a difficult dependency to satisfy.

However, it will require a C++20 compiler and cannot be compiled with clang 16 or earlier due to compiler bugs.

Restriction to C++20 and clang 16+, might be acceptable although not ideal. Maybe some of these bugs can be addressed?

I will allow the user to choose whether to build the current tester/iut and the new tester by cmake options.

If the user can choose between old and new tester then I suppose the C++ compiler dependency is not a huge issue.

@shibatch
Copy link
Owner Author

shibatch commented Oct 3, 2024

By the way we are currently reworking the benchmark suite using C++.

If so, could you briefly explain the plan in a new issue?

I think the existing testers could use a bit of documentation (inline and on website) to facilitate maintenance.

Yeah. We should add documentation across the board.

I still wonder what sort of environment does not have access to MPFR? This did not occur to me that this could be a difficult dependency to satisfy.

MPFR is quite troublesome to use on Windows.
I guess we will have to test our library on Windows on ARM, sooner or later.

@blapie
Copy link
Collaborator

blapie commented Oct 3, 2024

If so, could you briefly explain the plan in a new issue?

Sure, we will post about it in an issue shortly.

I guess we will have to test our library on Windows on ARM, sooner or later.

We have the ability to test on WoA internally. We work under Msys2 (which a unix-like environment) and it is relatively easy to install gmp, mpc and mpfr using the package manager pacman. The issue on WoA with Msys2 is the POSIX-dependency.

@shibatch
Copy link
Owner Author

shibatch commented Oct 3, 2024

Are you testing building SLEEF with MSVC?
MinGW seems to be imcompatible with MSVC.

@blapie
Copy link
Collaborator

blapie commented Oct 3, 2024

We are building with clang, it's proved simpler on many occasions.
Not saying there isn't a way to make it work, just still haven't got round to building successfully.

@shibatch
Copy link
Owner Author

shibatch commented Oct 4, 2024

In any case, using MPFR with MSVC is a bit tricky, and I think it would be ideal to test SLEEF properly with MSVC. The new tester should be useful for this purpose.

@blapie
Copy link
Collaborator

blapie commented Oct 4, 2024

Sure! Looking forward to the PR.

@joanaxcruz
Copy link
Contributor

joanaxcruz commented Oct 4, 2024

If so, could you briefly explain the plan in a new issue?

For now, we didn't create a issue as #9 is still open.
In this issue, you discuss introducing googlebench framework. I decided to investigate a bit on this, and the following points made it appealing to me:

  • active repo - https://github.com/google/benchmark
  • widely used in other projects
  • reduced maintenance cost - this would fall on the people maintaining the framework
  • straightforward integration with CMake
  • easy to use
  • comes with a lot of free functionality with it (like filtering the benchmarks to only show performance of a particular function).

Should upload a PR introducing this new tool soon - will keep you updated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants