search

BoostGSoC21 / math

Boost.org math module
http://boost.org/libs/math
Boost Software License 1.0
0 stars 1 forks source link

Universal complex type for FFT. #17

Closed cosurgi closed 3 years ago

cosurgi commented 3 years ago

In this PR I created file multiprecision_complex.hpp which implements solution to https://github.com/BoostGSoC21/math/issues/16 inside the boost::multiprecision namespace.

To make it work with many types I had to, temporarily, #include <boost/multiprecision/mpfr.hpp>. Solving this problem of having to include them is the focus of https://github.com/BoostGSoC21/math/issues/16

@ckormanyos , I need your help in getting the CI to work - namely to make sure that it finds mpfr.hpp. I tried to add in the test/Jamfile.v2 ../tools//mpfr ../tools//gmp. I will try to add this to other lines in there.

cosurgi commented 3 years ago

@Lagrang3 I did rename abstract_ring.hpp into multiprecision_complex.hpp since that was the only thing remaining in that file. Also I adapted all other parts of the code to these new features provided in multiprecision_complex.hpp.

cosurgi commented 3 years ago

OK. Looks like I got the CI to pass :)

ckormanyos commented 3 years ago

help in getting the CI to work

Thanks Janek for great progress. I will take a look ASAP and go through CI if/when/still needed.

Lagrang3 commented 3 years ago

Please check this! CI tests are not running due to the has_mpfr condition in the targets. Is it really necessary to compile with mpfr and gmp?

ckormanyos commented 3 years ago

Tests are not running.

I am about to look into this. Will try to correct it on the branch by installing the prerequisites in CI. Iprobably need to do something straightforward. Sorry about this delay...

Lagrang3 commented 3 years ago

I've added the mpfr and gmp prerequisites. But now Clang is failing due to lack of quadmath,

ckormanyos commented 3 years ago

But now Clang is failing due to lack of quadmath

clang will not have support for quadmath.

We should make another job in CI one for GCC/clang without quadmath. And a second for GCC with the files tested with quadmath

ckormanyos commented 3 years ago

This is in the YAML. We can look together or I can roll out a second job in CI having files not tested with quadmath running on clang

ckormanyos commented 3 years ago

This is in the YAML. We can look together or I can roll out a second job in CI

Hi @cosurgi and @Lagrang3 I get the feeling we need two test suites in the jamfile and two jobs in CI. I need a bit of time to straighten this out in CI. We also need to look at the type deduction and ensure that float128 is only being clled for when it is available in the given system.

I will look into these points...

ckormanyos commented 3 years ago

@Lagrang3 if you would like to move forward more quickly without being delayed by this, please feel free to simply remove clang++ from the test matrix in YAML at a line like here.

I can patch the multi-compiler stuff in the background while you move forward and I can indicate when I'm done.

ckormanyos commented 3 years ago

simply remove clang++ from the test matrix in YAML

I have done this temporary change and also appropriately modified the branch protection. Let's see how this runs through...

Lagrang3 commented 3 years ago

@Lagrang3 if you would like to move forward more quickly without being delayed by this, please feel free to simply remove clang++ from the test matrix in YAML at a line like here.

I can patch the multi-compiler stuff in the background while you move forward and I can indicate when I'm done.

OK.

ckormanyos commented 3 years ago

Hi @Lagrang3 and @cosurgi I did the separation and division in YAML/CI.

Along the way added some C++11 compatibility path(es) where needed. Hoping that CI on this branch goes green now...

Tests are cycling.

ckormanyos commented 3 years ago

help in getting the CI to work

Hi @cosurgi and @Lagrang3 based on the results of CI I refactored into:

I needed to do some tiny code refactoring for C++11/14 transition spot as well. I changed the branch protections to be matched up with the CI jobs in this PR.

So other branch(es) will need to align to this CI, or we need to discuss how to manage that if multiple branches need various CI job(s), which i would like to avoid if possible.

Anyway, this PR is going green in CI now.

ckormanyos commented 3 years ago

To me this is good to merge.

@Lagrang3 and @cosurgi we are all green now in CI. Is the above comment still correct? Should we merge to develop?

Lagrang3 commented 3 years ago

Yes. Merge to the real fft branch. I am now working in a homemade rfft for the boost backend.Thank you.

ckormanyos commented 3 years ago

Yes. Merge to the real fft branch.

Oh... Finally I see that. This PR does not want to merge to develop, but to the real-to-complex branch.

Lagrang3 commented 3 years ago

Hurray!

cosurgi commented 3 years ago

Thank you very much for fixing the CI pipeline. I will remember now to pay attention to the CI filters such as has_mpfr. I must admit that the gitHub CI is kind of new to me. I only have experience with gitLab CI methods, such as this one :). I should have looked in closer at this problem.

ckormanyos commented 3 years ago

for fixing the CI pipeline

Thanks Janek. I still need to resolve a few issues and should have a CI pipeline patch tomorrow.