Skip to content

BUG: increase precision#415

Closed
fedorov wants to merge 4 commits intoQIICR:masterfrom
fedorov:increase-precision
Closed

BUG: increase precision#415
fedorov wants to merge 4 commits intoQIICR:masterfrom
fedorov:increase-precision

Conversation

@fedorov
Copy link
Member

@fedorov fedorov commented Jan 27, 2021

resolves #414

@fedorov fedorov force-pushed the increase-precision branch from fe99d38 to c99143e Compare February 2, 2021 17:32
ostringstream sstream;
sstream << scientific << f;
return sstream.str();
sstream << fixed << setprecision(15) << f;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sstream.imbue(std::locale::classic());
before using the sstream.

return sstream.str();
sstream << fixed << setprecision(15) << f;
string formatted_f = sstream.str();
size_t dot_position = formatted_f.find('.');
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can move thin into the if statement as it is only used in that scope.

Use fixed notation precision 15 in place of scientific notation
default precision

resolves QIICR#414
@fedorov fedorov force-pushed the increase-precision branch from 8cb33bd to 6247fa3 Compare February 2, 2021 18:57
ostringstream sstream;
sstream << scientific << f;
return sstream.str();
sstream << fixed << setprecision(15) << f;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you use fixed, you should only use a precision of e.g. 9.
Reason using fixed percision only specifies the numbers after(!) the decimal point.
So currently you always produce to long strings.

9 digits after the decimal points would be enough in medical imageing cases are enough in my opinion and it is highly unlikely that you need more then 6 digits infront.

@fedorov fedorov force-pushed the increase-precision branch from 4cf93f2 to 393d72f Compare February 2, 2021 22:37
@kislinsk
Copy link
Contributor

kislinsk commented Feb 3, 2021

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.

@kislinsk
Copy link
Contributor

kislinsk commented Feb 3, 2021

Nevermind, I just noticed that DCMQI only used float so it makes things easier in the context of 16 characters. The standard library provides useful constants for serializing floating point numbers with maximum precision and for float (with all the extras) it always fits even into less than 16 characters. I will create a pull request for review.

@fedorov
Copy link
Member Author

fedorov commented Feb 8, 2021

replaced by #416

@fedorov fedorov closed this Feb 8, 2021
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.

DCM Seg: Rounding of orientation matrix when storing seg

3 participants