Page MenuHomePhabricator

Reopen segmentation in MITK
Closed, ResolvedPublic

Description

The problem is not anymore, that the data cannot be loaded (T28007). The workbench loads the dataset and the segmentation, but the segmentation cannot be changed, since the segmentation plugin seems to not identify the segmentation as belonging to the image (within "Select a segmentation" the segmentation cannot be found).
So I even tried the following steps ( in Windows and Linux):

  1. Open the dataset: E130-Personal:\Gao\mitk-flow-201216124223521692 (in REF_IMG)
  2. create a new segmentation with MITK.
  3. save the seg and close the project
  4. reopen image + seg
  5. even the segmentation created by MITK cannot be selected in the segmentation plugin anymore.

This could be geometry related: I tried it with an image of MITK-Data and could not reproduce it.
This is probably related to T28116, which is the same issue, but in the FlowBench

Revisions and Commits

rMITK MITK
Restricted Differential Revision
Restricted Differential Revision

Related Objects

StatusAssignedTask
Resolvedkislinsk

Event Timeline

gaoh triaged this task as High priority.Dec 21 2020, 9:57 AM
gaoh created this task.
gaoh raised the priority of this task from High to Unbreak Now!.Jan 13 2021, 10:06 AM

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

floca added a project: Request for Discussion.
floca added subscribers: fedorov, nolden.

The problem seems to be that DCMQI rounds the the orientation matrix values at the 5th decimal place when storing to DCM Seg.

Stored as nrrd:

space directions: (0.68359375,0,0) (0,0.68359375,0) (0,0,1)

Stored as DCM Seg:

space directions: (0.68359380000000003,0,0) (0,0.68359380000000003,0) (0,0,1)

Our sub geometry check is to pendantic as it only tolerates 1e-6. Changing the tolerance e.g. to something like 5e-5 gives enough tolerance and the data can be loaded.

@kislinsk / @nolden What do you think? How far can we relax the tolerance (NODE_PREDICATE_GEOMETRY_DEFAULT_CHECK_PRECISION) for geometry and sub geometry checks? It is not trival to find a default that is pragmatic enough to work well out there but still does a real check. But 5e-5 could be OK in normal medical imaging I think. That means that:

  • origin/spacing/size/corner points can diverge up to 0.00005 mm
  • values in the orientation matrix may also diverge up to 0.00005 (~ 0.003°).

@fedorov Andrey, do you know the reason why the rounding happens in DCMQI?

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.

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.

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 added a revision: Restricted Differential Revision.Jan 18 2021, 9:42 PM

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

gaoh moved this task from Restricted Project Column to Restricted Project Column on the Restricted Project board.Jan 20 2021, 10:32 AM
gaoh reopened this task as Open.EditedJan 20 2021, 9:44 PM

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

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 lowered the priority of this task from Unbreak Now! to Normal.Jan 27 2021, 3:25 PM

We're waiting for a fix of https://github.com/QIICR/dcmqi/issues/414 instead of further lowering precision in checks.

kislinsk removed kislinsk as the assignee of this task.EditedFeb 2 2021, 3:45 PM
kislinsk edited projects, added MITK, Next Milestone; removed Missing Info, MITK (v2021.02).

Fixed in release branch by patching DCMQI. I leave this task open as a reminder to switch back to an official DCMQI release as soon as they fixed it upstream.

@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.

kislinsk claimed this task.
kislinsk edited projects, added MITK (v2021.02); removed Next Milestone, MITK.

The fix was merged upstream and this task will be resolved with the merge of the current release branch this week.