Skip to content

Conversation

@cffk
Copy link
Contributor

@cffk cffk commented Jan 25, 2025

The old semantics:

MPREAL_HAVE_DYNAMIC_STD_NUMERIC_LIMITS: digits() is a function
!MPREAL_HAVE_DYNAMIC_STD_NUMERIC_LIMITS: digits is a constant

But in the latter case, the constant value of digits is hardwired into mpreal.h.

The new semantics:

MPREAL_FIXED_PRECISION == 0: digits() is a function
MPREAL_FIXED_PRECISION > 0: digits = MPREAL_FIXED_PRECISION

This makes it easy to select the fixed precision at compile time with, e.g.,

-DMPREAL_FIXED_PRECISION=256

At runtime

set_default_prec(numeric_limitsmprf::mpreal::digits);

should be called to make the actual precision consistent with digits.

The old semantics:

   MPREAL_HAVE_DYNAMIC_STD_NUMERIC_LIMITS: digits() is a function
  !MPREAL_HAVE_DYNAMIC_STD_NUMERIC_LIMITS: digits is a constant

But in the latter case, the constant value of digits is hardwired into
mpreal.h.

The new semantics:

  MPREAL_FIXED_PRECISION == 0: digits() is a function
  MPREAL_FIXED_PRECISION >  0: digits = MPREAL_FIXED_PRECISION

This makes it easy to select the fixed precision at compile time with, e.g.,

  -DMPREAL_FIXED_PRECISION=256

At runtime

  set_default_prec(numeric_limits<mprf::mpreal>::digits);

should be called to make the actual precision consistent with digits.
@cffk
Copy link
Contributor Author

cffk commented Jan 25, 2025

Sorry to be submitting this PR superseding the previous #27. It should
be uncontroversial. Perhaps the macro name MPFR_FIXED_PRECISION can be
changed. Also, I left it that the user needs to call set_default_prec to
the matching precision. (This is probably the right choice.)

cffk referenced this pull request Feb 5, 2025
Switch the MPREAL_HAVE_DYNAMIC_STD_NUMERIC_LIMITS logic from
defined/undefined to 1/0.  This make it easy to set the value
externally, e.g., with

  #define MPREAL_HAVE_DYNAMIC_STD_NUMERIC_LIMITS 0
  #include <mpreal.h>

I needed this facility using Boost's

  boost/numeric/odeint/stepper/bulirsch_stoer_dense_out.hpp

But most users won't notice this change.
@advanpix
Copy link
Owner

advanpix commented Feb 9, 2025

I appreciate the proposal and contribution, but I am not sure if it makes sense switching to the MPREAL_FIXED_PRECISION at the moment. First of all, this might break a lot of projects relying on mpreal (projects which are not maintained but actively used).

Secondly, introduction of MPREAL_FIXED_PRECISION may create confusion for most of the users. They might assume it controls default precision overall. I believe we should avoid such potentially ambiguous features, particularly regarding precision control.

The proper control of MPFR's default precision is not easy as it is. Basically there are two ways (build time/runtime):

  • Set the default precision in MPFR source code and build such customized version of MPFR.
  • Runtime control using mpfr_set_default_prec. Calling the function once somewhere at the start is not enough. MPFR maintains its own precision per thread and mpfr_set_default_prec must be called in each thread, e.g. in OpenMP:
        #pragma omp parallel firstprivate(precision, N)
        {
            mpfr_set_default_prec(precision);

            #pragma omp for schedule(static)
            for (int i = 0; i < N; i++)
            {

            }
            mpfr_free_cache(); 
        }

IMHO, adding another entity which might be confused with precision control will make things more complicated (even now most emails I receive from users are related to precision control).

Now, in case of MPREAL_HAVE_DYNAMIC_STD_NUMERIC_LIMITS, mpreal provides true precision in all circumstances (including threads) and falls back to default MPFR precision if macro disabled. This is far from perfect but is minimum and transparent functionality C++ class can provide without adding too much confusion (?)

Boost, Eigen, etc. probably need to introduce customized numeric traits allowing digits to be dynamic, if arbitrary precision support is planned.

I would appreciate comments & opinions.

As an idea - maybe we should remove the numeric_limits<mprf::mpreal> from core mpreal.h altogether and leave its implementation to user? The current implementation of numeric_limits<mprf::mpreal> will be provided in a different header (as an optional default/example) - and users can customize it in any way possible.

@cffk
Copy link
Contributor Author

cffk commented Feb 9, 2025

Let me give my use case:

I routinely compile my codes to use double, long double, boost::multiprecision::float128, and mpfr::mpreal as the default precision. I typically use 256 as the precision for mpfr::mpreal and I am aware of the need to set the precision at the start of every thread of my program.

I recently needed to use Boost's dense solvers for ODEs and these require that numeric_limits<mpreal>::digits be a constant. So in the first pass, I set MPREAL_HAVE_DYNAMIC_STD_NUMERIC_LIMITS to 0 and edited mpreal.h to set numeric_limits<mpreal>::digits to 256. But this is unsatisfactory because I have to edit mpreal.h any time I want to change my precision.

This PR addresses this problem by replacing the magic constant 53 in mpreal.h by MPREAL_FIXED_PRECISION which can be set at compile time; this obviates the need to edit mpreal.h.

I concede that some users may be confused. But only if they set MPREAL_FIXED_PRECISION and I think a stern warning in the documentation about setting the default precision accordingly would be enough.

Beyond this, I could imagine modifying the code so that for MPREAL_FIXED_PRECISION > 0:

  • set_default_prec checks that the requested precision matches MPREAL_FIXED_PRECISION.

  • the constructors for mpreal check that set_default_prec matches MPREAL_FIXED_PRECISION.

I don't see the need to these changes and would trust the users to know what they are doing.

@cffk
Copy link
Contributor Author

cffk commented Feb 12, 2025

I should add, that while I think this PR is, on balance, useful, I'm fine with it not being accepted. I can work with my patched version of mpreal.h. The software I distribute doesn't depend on boost and so doesn't need this PR to be applied.

@advanpix
Copy link
Owner

Beyond this, I could imagine modifying the code so that for MPREAL_FIXED_PRECISION > 0:

  • set_default_prec checks that the requested precision matches MPREAL_FIXED_PRECISION.
  • the constructors for mpreal check that set_default_prec matches MPREAL_FIXED_PRECISION.

This is exactly what I want to avoid in mpreal. In my view, mpreal should stay thin and simple wrapper over MPFR.

Advanced precision control (together with things like optimized memory allocators for mpreal, MPFR cache release, etc.) should be left for the user. Also numeric traits shouldn't be a part of mpreal. User can supply its own traits depending on the library it uses, he/her likings, etc.

Thus, I consider inclusion of std::numeric_limits<mpreal> specialization was a mistake. Most probably, I will remove it from the mpreal in the future. Therefore I am not sure if it makes sense to merge the pull request at the moment.

In any case, thank you very much for all your suggestions and contributions!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants