Add std::complex<long double> Support

[ax3l]

Implement support for the std::complex< long double > type.
We need this downstream in openPMD-api and it's generally a missing type (since we also have a type for long double).

Help wanted :)

To Do

• BP3 engine: BP3 only supported float and double complex: anything to do?
• test updates (partial)
• update bindings
  • Fortran (also adds missing long double type): [WIP] Fortran: Real16 (complex) Support #1921
  • Matlab (no fundamental long double type?)
  • C (also adds missing long double type)
  • Python

As a follow-up, a native ADIOS bool type would be great for C++ codes, too.

[ax3l]

@germasch Can you please take a look if I updated the C bindings completely and properly with the new type? I also added the missing support for long double in those.

[germasch]

I don't think I'm specifically responsible for the C bindings (all I did was improve handling of strings there at some point), but anyway, those changes there look good to me.

I have some concerns about "long double" in general, though. It's not a standard type, and the way that I'm familiar with it is the 80-bit x87 floating point type, which has pretty much outlived its usefulness, I thought, as most compilers will by default use 64-bit SSE/AVX instructions when dealing with it, anyway. That's not to say that I'm opposed to providing support for it if it's needed by some application.

I don't think it should be called "r128", though, since at least on the most common arch, it's not 128-bit. In fact, one of the things I did early on is kinda put a boundary between C++ primitive types on one hand (the C/C++ interface) and stdint types (used internally). E.g., unsigned long can be 32 or 64 bit depending on system, and so while the C++/C interface take unsigned long, the adios2 will internally map it to u32 or u64, and hence handle its actual size without regard to which system you're reading/writing on.

So if one wants to support longer than 64-bit precision, one should introduce probably both "r80" and "r128" separately,. Because otherwise things will go wrong when reading long double on a system where it's 80 bit, while having written it on a system that's 128 bit (it looks like that may be the case on IBM Power -- I'm in China and have trouble googling.)

[germasch]

So FWIW, I was wrong that long double is not a standard type -- it is, at least in C. But it's still true that it doesn't have a well defined bit width, so it's not good to use internally because things will break when moving files between systems.

Also, I had thought long double support was introduced by this PR, but after looking at it, I now realize it's already there, just not for attributes. In other words, the lack of well-defined bit width is a pre-existing issue, not one introduced by this PR.

[ax3l]

Thank you for the review! I just saw your name in several of the C binding files and assumed you authored a larger part of it.

The length handling of ADIOS is generally not fixed size, simply by the definition that in C/C++ (short float) <= float <= double <= long double.
I am not entirely sure if adios just transparently maps to the platform size when moving data between different data models or if this is a portability issue, but as you said already above this is fully independent of this PR.

In terms of variable namings, which are not critical, I just used the default size on x86 64bit linux platforms as assumed in all other places for float and double as well.

[germasch]

The length handling of ADIOS is generally not fixed size, simply by the definition that in C/C++ (short float) <= float <= double <= long double.

What happens for integer is that the C++ primitive type (say unsigned long) internally gets mapped to u64 (uint64_t, really) or u32, depending on what its actually width on the given system is. So it will be stored with its actual precision. If you store an unsigned long on a 64-bit system, it'll be stored with 64 bits. If you open that file on a 32 bit system, you cannot access that variable as an unsigned long because that's only 32 bits. You could access it as an unsigned long long, though.

For float and double, no such mapping actually happens, though it probably should. The thing is, though, that on all current systems that I'm familiar with, float is float32 and double == float64, so because of that explicitly mapping them hasn't been needed. However, for long double is highly system / compiler dependent, and hence its use is not portable. This is kinda my fault in the sense that back then I fixed the integer types, but not the floating point types.

I am not entirely sure if adios just transparently maps to the platform size when moving data between different data models or if this is a portability issue, but as you said already above this is fully independent of this PR.

Yes, it's preexisting to this PR, though this PR exposes the problem in additional ways. I'm not saying it's a showstopper, but it'd probably be good to think about what to do with long double in the first place, independently of this PR. The options that I see are

1. leaving things as they are, ie, declare use of long double to lead to non-portable files (though it worth nothing that reading a file that has long double in it on another system can cause buffer overflows, a.k.a, weird crashes and security vulnerabilities)
2. drop support for long double altogether (and possible add support for __float128 if needed by some app.)
3. fix long double support, but it looks pretty painful, because on wikipedia it looks like long double can be (a) a 64-bit float (b) an 80-bit float stored in 96 bits (c) an 80-bit float stored in 128 bits (d) a "double-double", or (e) an actual 128-bit float, depending on system and compiler (and compiler options). Fun...

Maybe most notably, apparently with MSVC long double is just a 64-bit float, most definitely not compatible with the Linux/x86_64 long double. I just tried Summit vs Linux/x86_64, and while both have storage size of 128 bits for long double, the actual binary representation is different. I suppose using this PR you could try writing a long double attribute on Summit and read it on x86_64, and you'll see that you get the wrong answer.

In terms of variable namings, which are not critical, I just used the default size on 64bit linux platforms as assumed in all other places for float and double as well.

Unfortunately, even on 64-bit Linux, the binary representation of long double (nor the actual precision) is not consistent (ie, on ppc64 != on x86_64), though at least they're all stored in 16 bytes.

[ax3l]

What happens for integer is that the C++ primitive type (say unsigned long) internally gets mapped to u64 (uint64_t, really) or u32, depending on what its actually width on the given system is.

I thought so as well for long, but it's exactly the other way around.
short, int, long, long long (+ their unsigned variants) are the fundamental integer types, even if some of them implement the same size, all others are (system-dependent) typedefs.
Ref.: https://en.cppreference.com/w/cpp/language/types (and actual headers of your stdlib)

Implementing the fundamental types in the user-facing API overloads, as now, is generally the right thing to do. Implementing just the fixed (alias) types leads to problems, namely that some types a user provides will not map overloads. Assuming the type is known/identical between systems is not good on the file format level, but I don't even know if this is happening here. Nevertheless, this is all very off topic for this PR.

[germasch]

I thought so as well for long, but it's exactly the other way around.
short, int, long, long long (+ their unsigned variants) are the fundamental integer types, even if some of them implement the same size, all others are (system-dependent) typedefs.
Ref.: https://en.cppreference.com/w/cpp/language/types (and actual headers of your stdlib)

I know that -- I guess I wasn't clear. ADIOS2 internally maps the C++ primitive types (short, int, ...) to stdint types. And yes, those stdint types themselves are in fact typedefs for primitive types. The point however, is that it takes the ambiguity away. On x86_64, both unsigned long and unsigned long long are mapped to uint64_t, (and I think uint64_t is in fact unsigned long, but that might be system dependent and doesn't matter for adios2). The point is that the type is written to disk marked as "u64", no matter what it originally was. And when you open a Variable<unsigned long> or a Variable<unsigned long long> (which are distinct types) they both again get mapped internally to core::Variable<uint64_t>, so either of those can be used to read the u64 data back.

Implementing the fundamental types in the user-facing API overloads, as now, is generally the right thing to do. Implementing just the fixed (alias) types leads to problems, namely that some types a user provides will not map overloads.

Yes, even though there was some discussion about that at the time, that's the approach I pushed for, and that got adopted eventually. Obviously, when you support all primitive types, the stdint types, which are aliases for a subset of the primitive types, work just fine as well. Before that change, essentially only the stdint types themselves (and whatever they happen to alias to) were properly supported, and as you say, that lead to problems.

Assuming the type is known/identical between systems is not good on the file format level, but I don't even know if this is happening here.

That's why it maps to the stdint types internally, which are identical on the filesystem level. The same can't be said of long double, though (but that's not this PR's fault).

[ax3l]

Ha, the PGI Fortran compiler still does not support quad precision. Welcome to 2020.

[ax3l]

@guj this seams to write some H5Tclose(): not a datatype errors to stderr in some tests.

[ax3l]

Updated C++/C/Python frontend to soley refer to the missing fundamental (complex) long double types. "Serialization" (aka portability of fundamental types) is not touched by this PR and is an independent task that needs to be generally fixed for the BP file format. The problem is not new with long double and already existed with e.g. short/int/long/long long integer types.

I moved the Fortran binding updates for real16 kinds to a follow-up PR in #1921.

This should allow us to review (and potentially merge) this PR now.

@chuckatkins please remove the api: Fortran label herein :)

[germasch]

FWIW, this alleviates my main concern, ie., the Fortran bindings that use real*16, which is != long double even on the same system are gone from this PR.

It looks like even though you separated out the Python interface changes into a separate PR #1922, that PR is missing most of the Python-related updates (the python tests in particular), while this PR still has all of the Python changes, so if you want them to be separate, you should still remove them from here. (I didn't look deeply into the Python changes, but I don't think I have fundamental concerns about them -- np.longdouble is the same as C/C++ long double, ie, consistent on the same system, but not portable, which is what the status of `long double in ADIOS2 is in general.)

[ax3l]

Agree.
FWIW, #1922 is only a follow-up to the already merged complex support as attributes and is independent of this PR, which adds new types. Yes, additional (already passing) tests are added when we merge this again, I'll rebase this PR anyway as soon as #1922 is merged.

[ax3l]

@williamfgc @pnorbert @guj ready for review :)

[williamfgc]

@ax3l are you handling the PGI compilation issue in this PR? We saw this on Summit #814 and I want to make sure this PR is not bring it back. Thanks!

[ax3l]

I didn't see an issue with PGI but relied on CI for that, which passed.
I thought #814 is about GCC 6.4.0? Cannot see any problem with that locally or in CI.

Update: just tried to compile on Summit with GCC 6.4.0 which results in

In file included from /autofs/nccs-svm1_sw/summit/gcc/6.4.0/include/c++/6.4.0/bits/char_traits.h:39:0,
                 from /autofs/nccs-svm1_sw/summit/gcc/6.4.0/include/c++/6.4.0/string:40,
                 from /autofs/nccs-svm1_sw/summit/gcc/6.4.0/include/c++/6.4.0/bitset:47,
                 from /ccs/home/huebl/src/adios2/source/adios2/toolkit/format/bp/BPBase.h:14,
                 from /ccs/home/huebl/src/adios2/source/adios2/toolkit/format/bp/BPBase.cpp:11:
/autofs/nccs-svm1_sw/summit/gcc/6.4.0/include/c++/6.4.0/bits/stl_algobase.h: In member function 'void adios2::format::BPBase::ParseCharacteristics(const std::vector<char>&, std::size_t&, adios2::format::BPBase::DataTypes, bool, adios2::format::BPBase::Characteristics<T>&, bool) const [with T = std::complex<long double>]':
/autofs/nccs-svm1_sw/summit/gcc/6.4.0/include/c++/6.4.0/bits/stl_algobase.h:368:6: internal compiler error: in convert_move, at expr.c:644
      __builtin_memmove(__result, __first, sizeof(_Tp) * _Num);

@chuckatkins do you potentially want to add Travis-CI's new PPC64le architecture to CI?

[ax3l]

This seams to be an internal GCC 6.4.0 issue which is fixed on Summit/PPC64le with GCC 7.4.0, 8.1.0, 8.1.1, 9.1.0 - these are the versions I tested now.

[williamfgc]

Ha, the PGI Fortran compiler still does not support quad precision. Welcome to 2020.

@ax3l I was making reference to your comment above wrt PGI, thanks for checking that's not an issue.

After reviewing the topic we decided to go for a more robust solution to map currently available non-standard types. @chuckatkins will kickstart the work on the CMake side and we can propagate to BP as in this PR. We can use parts of this PR or build on top. Thanks for double-checking that there are outstanding issues with gcc <= 6.4.0. My preference is not to merge this PR as-is (breaks master compilation with gcc <= 6.4.0 which is still widely used), but keep it open until mapping the non-standard long double with the corresponding (non-standard) fixed width type is ready. @chuckatkins please keep us posted. Thanks!

[pnorbert]

@ax3l In yesterday's dev meeting the team agreed to do the following (which will be done for the next release)

• Introduce the specific floating point types into the BP format (thanks @germasch for pointing out a lot of those types)
• CMake will determine at configuration time which types are available for the specific architecture/language/compiler combo
• 'long double' will be a supported non-standard type in the ADIOS C++ interface like 'size_t' is now, and will translate to a specific type in each ADIOS build (so an application may not be portable across machines).

We thought accepting this PR as is so you can move on, and then do the above as updates but the requirement to use GCC 7 or newer scares us. Can we hold on to this PR for a while, until we have the CMake magic (@chuckatkins) to get the support for long double and conditional compilation to avoid breaking ADIOS with GCC 5 and 6?

A followup development later will be to implement conversions between types as Operators and add them automatically when application type does not match data type in input.

[ax3l]

Ha, the PGI Fortran compiler still does not support quad precision. Welcome to 2020.

I was making reference to your comment above wrt PGI, thanks for checking that's not an issue.

Ah I see, no that's only a Fortran binding issue and has been moved to #1921 and got a CMake feature test and pre-compiler guards.

[ax3l]

Thanks @williamfgc and @pnorbert for the updates!
Your plan sounds good to me, please feel free to scavenge this PR as needed.

[ax3l]

Just to update this issue: I guess the general goal should be multi-precision float support, especially for half-precision types (bfloat & float16) which already landed in the development branch of e.g. OpenMPI and are actively used in math libraries and apps already. Support for long double is just a very similar problem.

[germasch]

It looks to me like this got closed when @chuckatkins pulled in the upstream pybind11, which fixed a pybind11 issue which happened to have the same issue number.

[ax3l]

@chuckatkins @pnorbert I forgot what the status on this is. Is there any progress on how to add std::complex<long double> to complete the type model (similar to the handling of long double in ADIOS2)?

[eisenhauer]

@ax3l We're trying to close old, impossible-to-merge-as-is PRs. It looks like there were action items on this that never actually got addressed. CMake issues for @chuckatkins, maybe something for @pnorbert... The comment on half-precision types is likely something we should be thinking about for GPUs too (@anagainaru) . I think those are unlikely to be addressed prior to our next release, but it's also the case that this PR is applicable to a two-year-old code base and developments like BP5 change the landscape significantly. Can I suggest closing this PR and maybe creating a summary Issue (if these concerns aren't adequately represented in an issue already)?