Oh I forgot to mention: the segmentation is saved as dicom-seg.

Thanks for looking into this issue as well. :-) I think 1e-5 is okay. We/you already introduced NODE_PREDICATE_GEOMETRY_DEFAULT_CHECK_PRECISION not long ago since we had similar issues and 1e-5 should still be conservative enough for the purpose of geometries.

In T28127#217306, @kislinsk wrote:

1e-5 should still be conservative enough for the purpose of geometries.

Is your statement also valid for 5e-5? Beause 1e-5 would not be enough in that case.

It depends. NODE_PREDICATE_GEOMETRY_DEFAULT_CHECK_PRECISION is only for checking in predicates; not for constant use throughout MITK while are there calculations after calculations after calculations and so the values would drift away further from the actual truth, right? So for a temporary fix, I'm okay. But we should definitely check what is going on in DCMQI and fix on that side preferably.

In T28127#217365, @kislinsk wrote:

It depends. NODE_PREDICATE_GEOMETRY_DEFAULT_CHECK_PRECISION is only for checking in predicates;

As long as nobody has missused it so far, it is just used for that. ;) At least I have introduced it with this purpose and only used it there.

Have opened an DCMQI issue (https://github.com/QIICR/dcmqi/issues/414) to clarify why we could ensure the best fidelity when storing DCMQI.

@floca thanks for the report! I added a comment to the dcmqi issue you created.

In T28127#217604, @gaoh wrote:

@floca I might have found a dataset, where even the new number is not enough:
P:\Gao\mitk-testdata\

Whats P:?

@kislinsk I think we have to look again in the patch. Andrey and I had a conversion because even if the standard says otherwise he had problems with resulting invalid dicom representtions (longer then 16 bytes). May be you have an idea. Have a look on the git hub thread. Andrey changed it now back to scientific with precision 8, but this would mean we are still not on the same side. On the other hand I do not want to risk producing invalid dicom (what our patch produces). So I think in the end the best way, might be to ensure it by hand. So check after stream conversion, if we have more then 16 characters. If this is the case give a warning and truncate so that it is a valid DICOM decimal string. What do you think?

Here's the PR with CI testing results https://github.com/QIICR/dcmqi/pull/415. You can look at the CI results for earlier commits, if you would like to investigate.

@floca When DICOM says 16 characters/bytes, does it include a trailing \0 character or is it really a fixed array of size 16?

Here's my shot: http://cpp.sh/72ipr

To squeeze every little possible bit of precision out of it, you could print both scientific and fixed notation to strings and compare their lengths. If the scientific notation fits, use it. Fixed notation otherwise. The numeric_limits::max_digits10 ensures that both notations represent the maximum possible precision.

Oh, it's even simpler. I see that in DCMQI it is only float. max_digits10 is 9 for float and according to the documentation its purpose is exactly for serializing and deserializing floating point numbers without losing any precision, even though the intermediate text representation may not be exact.

It boils down to:

string Helper::floatToStrScientific(float f) {
  ostringstream sstream;
  sstream.imbue(std::locale::classic());
  sstream.precision(std::numeric_limits<float>::max_digits10);
  sstream << f;
  return sstream.str();
}

However, the name of the function does not fit anymore so I will propose to change it to floatToStr(), as a quick scan through the DCMQI revealed that its actual purpose is to get a string out of a float for serialization.