OpenFOAMReader precision hardcoded to 32-bit

Hey guys,

I noticed that the OpenFOAM reader in VTK always reads mesh and field data as 32-bit.
I had a look at the vtkOpenFOAMReader.cxx file and it appears to that the conversion to 32-bit is hardcoded. Fields (ScalarListPtrs, VectorListPtrs) and point coordinates (pointArray) are always cast to vtkFloatArray regardless of the precision of the OpenFOAM results.

I am guessing this is semi-intentional and works fine for small/medium cases, but it becomes a genuine problem for larger data sets where the numerical truncation results into certain filters behaving unexpectedly and yielding low quality results. Moreover, this pours over to the Python VTK wheels, pyvista and ParaView.

Would you consider adding full support for 64-bit or is this out of the question?

FYI In OpenFOAM precision is usually controlled by the writePrecision variable in controlDict. Not sure if it would be just simpler to replace floats with doubles (although this would break backwards compatibility).

Of course, would you consider contributing that improvement ? :slight_smile:

1 Like

I can definitely try. It is a rather invasive change btw since it touches most parts of the reader.
I am guessing using 64bit and dropping the 32bit casting is out of the question, correct?

Correct, it must be optional I’m afraid.

FYI @olesenm

1 Like

@gnikit

Indeed. There are many use cases (eg, external aerodynamics) for which full double-precision is unnecessary and incurs a sizable memory overhead.
I suppose the first place to start looking might be at the MakeScalarList vs MakeLabelList (around Line 3590 in vtkOpenFOAMReader.cxx) as well as the ReadNonUniformListhandling. In both cases, you’d have an additional combination of io.isFloat64()coming from the input file as well as something like noNarrowFloatto indicate that the VTK field should also remain as double.

I haven’t had the time or need to actually work on this, but it feels like reasonably OK. For the first coding, can also just supply a file-scope bool constant to get going.

1 Like

The writePrecision variable dictates how many decimal places an ASCII formatted output should use. It doesn’t influence binary files at all. The actual single/double precision of the OpenFOAM output files is compile-time dependent and is indicated in file headers

arch "LSB;label=32;scalar=64"
This same information is used by OpenFOAM itself to manage reading files with different precision.

1 Like

Yeah, this is a good point. OpenFOAM itself controls numerical precision through its environment setup. However this does not necessarily mean that the active OF env is the one that generated a case, or even that an OF installation is present when the data is being read.

My first instinct is to fully decouple the VTK reader from the OF environment setup, writePrecision, etc. A bool option is added in the VTK API e.g. useDoublePrecision (or whatever) that will choose to use vtkDoubleArrays instead of vtkFloatArrays.

Focusing on reals and not integers for the time being, this would result in 4 combinations for Binary Inputs:

Combination Index Input Binary precision Cast Precision
0 32-bit 32-bit
1 32-bit 64-bit
2 64-bit 32-bit
3 64-bit 64-bit

NOTE: combination indices 1 and 2 are of questionable use IMO.

and 2 combinations for ASCII inputs; 32-bit and 64-bit cast precision.

Does this match what you had in mind? I can try and get a PR later today (hopefully).

Focusing on reals and not integers for the time being, this would result in 4 combinations for Binary Inputs:

My previous post was not making any assumption about OpenFOAM being installed or having any OpenFOAM environment at all, also not really caring about the contents of controlDict much either.

This is what you can expect to actually be on disk:

  • arch “label=(32|64);scalar=(32|64)”

In some cases, the arch entry may even have ...;scalar=32;solveScalar=64, but in those cases the solve-scalar is only used for the linear solve and not the field storage, which remain floatand not double.

So only considering OpenFOAM and VTK, you have essentially have the choice to read in OpenFOAM files that are float/double and store them in VTK as float/double, respectively (which is currently not the case), or do read OpenFOAM files (float/double) and store them in VTK as float (which is what the current reader does). So using your 0-3 combination indices, we currently have 32bit→32bit and 64bit →32bit (ie, combination 0, 2). Adding in a single bool switch (eg, “keepNativePrecision”), you would still have 32bit→32bit but also enable 64bit→64bit (combination 3). As far as OpenFOAM and VTK are concerned, this is all you need. So I guess your question is more about other consumers of the VTK data that require double. In this case, it’s not a binary choice but an additional choice to force target 64-bit output too? A selection of (float/double/native) representation.

enum class FloatHandling { Float, Double, Native, Legacy = Float };

if (io.IsFloat64())  // This is what the input files are telling us
{
    if (floatHandling == FloatHandling::Float)
    {
        // narrow double to float (current default)
        this->ReadNonUniformList<SCALARLIST, // 
            vtkFoamRead::listTraits<vtkFloatArray, double>>(io); 
    } 
    else
    {
        // retain double precision
        this->ReadNonUniformList<SCALARLIST, // 
            vtkFoamRead::listTraits<vtkDoubleArray, double>>(io); 
    }
}
else 
{
    if (floatHandling == FloatHandling::Double)
    {
        // Force widening from float to double
        this->ReadNonUniformList<SCALARLIST, // 
            vtkFoamRead::listTraits<vtkDoubleArray, float>>(io); 
    }
    else
    {
        // retain float precision (current default behaviour)
        this->ReadNonUniformList<SCALARLIST, // 
            vtkFoamRead::listTraits<vtkFloatArray, float>>(io); 
    }
}

Take your time…

1 Like

I’ve started having a crack at this but I think the precision issue is more widespread than originally thought. Getting the fields and mesh points to optionally be 64bit seems okay, but I am starting to spot forced downcasts in the tokenizer and a few other places. For example in
``

void vtkFoamEntry::Read(vtkFoamIOobject&io)
{
  ...
          else if (lastValue.Dictionary().GetType() == vtkFoamToken::SCALAR)
          {
            const float val = lastValue.Dictionary().GetToken().ToFloat();
  ...
}

There’s quite a few other places where floats are explicitly used. I am unsure of the wider effect of these downcasts. For now I’ll focus on the fields/mesh but I thought I should mention it.

There are indeed some places like that too. In the vtkFoamToken tagged union, floating point is actually stored as doubleinternally and integral as int64, so that’s not a limiting factor. For field “dimensions”, float resolution is correct and adequate. For several others, it could be an unnecessary cast.

See how far you manage with the mesh and fields and just add a comment like /* TBD with opencfd */in your draft code changes for the questionable bits and then we can go through them together.

NB: you may wish to handle the mesh points slightly differently too. In vtkPoints the default representation is float (not double) and is also adequate for many types of meshes.

cheers,

/mark

2 Likes

Thanks @olesenm and @mwestphal for all the feedback. I have a “working” patch.
I will document my choices and thinking on the PR. It should be live soon

I opened an MR here https://gitlab.kitware.com/vtk/vtk/-/merge_requests/12885
Please don’t hold back! Again thanks for all the help!