Skip to content

BUG: Validate Bruker geometry array cardinalities #6990

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jdotc1 wants to merge 1 commit into
base: main
Choose a base branch
from
+124 −7
Open

BUG: Validate Bruker geometry array cardinalities #6990

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 29 additions & 7 deletions Modules/IO/Bruker/src/itkBruker2dseqImageIO.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,23 @@ GetParameter(const itk::MetaDataDictionary & dict, const std::string & name)
return value;
}

std::vector<double>
GetParameterArray(const MetaDataDictionary & dict,
const std::string & name,
const std::vector<double>::size_type minimumSize)
{
auto values = GetParameter<std::vector<double>>(dict, name);
if (values.size() < minimumSize)
{
itkGenericExceptionMacro("Bruker parameter " << name << " has " << values.size() << " values, need at least "
<< minimumSize);
}
return values;
}

constexpr std::vector<double>::size_type numberOfPositionValues{ 3 };
constexpr std::vector<double>::size_type numberOfOrientationValues{ 9 }; // 3x3 matrix

// Parameter may be absent, hold a single value for all frames, or one value per frame
std::vector<double>
ReadDoubleArray(const MetaDataDictionary & dict, const char * name, double defaultValue)
Expand Down Expand Up @@ -973,10 +990,15 @@ Bruker2dseqImageIO::ReadImageInformation()
itkExceptionMacro("VisuCoreByteOrder parameter is invalid: " << byteOrder);
}

const SizeType brukerDim = static_cast<SizeType>(GetParameter<double>(dict, "VisuCoreDim"));
const double dimension = GetParameter<double>(dict, "VisuCoreDim");
if (dimension != 1.0 && dimension != 2.0 && dimension != 3.0)
{
itkExceptionMacro("VisuCoreDim must be 1, 2, or 3, got " << dimension);
}
const SizeType brukerDim = static_cast<SizeType>(dimension);
const SizeType frames = static_cast<SizeType>(GetParameter<double>(dict, "VisuCoreFrameCount"));
const auto size = GetParameter<std::vector<double>>(dict, "VisuCoreSize");
const auto FoV = GetParameter<std::vector<double>>(dict, "VisuCoreExtent");
const auto size = GetParameterArray(dict, "VisuCoreSize", brukerDim);
const auto FoV = GetParameterArray(dict, "VisuCoreExtent", brukerDim);

if (brukerDim == 1)
{
Expand All @@ -990,7 +1012,7 @@ Bruker2dseqImageIO::ReadImageInformation()
}
else
{
const auto position = GetParameter<std::vector<double>>(dict, "VisuCorePosition");
const auto position = GetParameterArray(dict, "VisuCorePosition", numberOfPositionValues);
// Bruker 'origin' is corner of slice/volume. Needs shifting by half-voxel to be ITK origin
// But for 2D images, the slice position is correct (center of slice)
vnl_vector<double> halfStep(3);
Expand Down Expand Up @@ -1055,7 +1077,7 @@ Bruker2dseqImageIO::ReadImageInformation()
if (spacingZ == 0)
{
// FrameThickness excludes the slice gap, so it is only a fallback
spacingZ = GetParameter<std::vector<double>>(dict, "VisuCoreFrameThickness")[0];
spacingZ = GetParameterArray(dict, "VisuCoreFrameThickness", 1)[0];
}
halfStep[2] = 0; // Slice position will be correct
}
Expand All @@ -1073,7 +1095,7 @@ Bruker2dseqImageIO::ReadImageInformation()
this->SetDimensions(3, sizeT);
if (dict.HasKey("VisuAcqRepetitionTime"))
{
const double TR = GetParameter<std::vector<double>>(dict, "VisuAcqRepetitionTime")[0];
const double TR = GetParameterArray(dict, "VisuAcqRepetitionTime", 1)[0];
this->SetSpacing(3, TR / 1e3); // TR is in milliseconds, convert to seconds
}
else
Expand All @@ -1087,7 +1109,7 @@ Bruker2dseqImageIO::ReadImageInformation()
// It is possible for every slice to have a different orientation,
// but ITK doesn't support this so concatenate all slices as if they
// had the same orientation
const auto orient = GetParameter<std::vector<double>>(dict, "VisuCoreOrientation");
const auto orient = GetParameterArray(dict, "VisuCoreOrientation", numberOfOrientationValues);

// The Bruker orient field is scanner-to-image. ITK is image-to-scanner.
// However, ITK stores column-wise, Bruker row-wise. So the below is
Expand Down
95 changes: 95 additions & 0 deletions Modules/IO/Bruker/test/itkBruker2dseqImageIOGTest.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,103 @@ ExpectReadThrows(const std::string & name, const std::string & visu)
reader->SetFileName(path);
EXPECT_THROW(reader->Update(), itk::ExceptionObject);
}

// Returns the description of the exception from ReadImageInformation, or an empty string if none is thrown
std::string
ReadImageInformationError(const std::string & path)
{
const auto io = itk::Bruker2dseqImageIO::New();
io->SetFileName(path);
try
{
io->ReadImageInformation();
}
catch (const itk::ExceptionObject & error)
{
return error.GetDescription();
}
return {};
}
} // namespace

TEST(Bruker2dseqImageIO, RejectShortGeometryArrays)
{
struct Case
{
const char * Name;
const char * Values;
unsigned int Count;
unsigned int Dimension;
};
const std::vector<Case> cases{
{ "VisuCoreSize", "", 0, 2 }, { "VisuCoreSize", "4", 1, 2 },
{ "VisuCoreSize", "4 2", 2, 3 }, { "VisuCoreExtent", "", 0, 2 },
{ "VisuCoreExtent", "4", 1, 2 }, { "VisuCoreExtent", "4 2", 2, 3 },
{ "VisuCorePosition", "", 0, 2 }, { "VisuCorePosition", "0 0", 2, 2 },
{ "VisuCoreOrientation", "", 0, 2 }, { "VisuCoreOrientation", "1 0 0 0 1 0 0 0", 8, 2 },
{ "VisuCoreFrameThickness", "", 0, 2 }, { "VisuAcqRepetitionTime", "", 0, 2 }
};
unsigned int caseNumber = 0;
for (const auto & testCase : cases)
{
SCOPED_TRACE(testCase.Name);
SCOPED_TRACE(testCase.Count);
std::string visu = MakeVisuPars();
const auto replaceRecord = [&visu](const std::string & name, const std::string & value) {
const std::string prefix = "##$" + name + "=";
const auto begin = visu.find(prefix);
if (begin == std::string::npos)
{
return false;
}
const auto end = visu.find("\n##", begin);
if (end == std::string::npos)
{
return false;
}
visu.replace(begin, end - begin, prefix + value);
return true;
};
if (testCase.Dimension == 3)
{
ASSERT_TRUE(replaceRecord("VisuCoreDim", "3"));
// The non-target geometry axis array must itself be valid.
const char * other = std::string(testCase.Name) == "VisuCoreSize" ? "VisuCoreExtent" : "VisuCoreSize";
ASSERT_TRUE(replaceRecord(other, "( 3 )\n4 2 1"));
}
if (std::string(testCase.Name) == "VisuCoreFrameThickness")
{
// Force the thickness fallback without a measured inter-slice step.
ASSERT_TRUE(replaceRecord("VisuCorePosition", "( 3, 3 )\n0 0 0 0 0 0 0 0 0"));
}
ASSERT_TRUE(replaceRecord(testCase.Name, "( " + std::to_string(testCase.Count) + " )\n" + testCase.Values));
const auto path = WriteDataset("bruker_short_geometry_" + std::to_string(caseNumber++), visu, 12);
ASSERT_FALSE(path.empty());
const std::string message = ReadImageInformationError(path);
ASSERT_FALSE(message.empty()) << "Short geometry array was accepted";
EXPECT_NE(message.find(testCase.Name), std::string::npos);
EXPECT_NE(message.find("need at least"), std::string::npos);
}
}

TEST(Bruker2dseqImageIO, RejectUnsupportedGeometryDimension)
{
for (const std::string dimension : { "0", "2.5", "4" })
{
SCOPED_TRACE(dimension);
std::string visu = MakeVisuPars();
const std::string record = "##$VisuCoreDim=2";
const auto position = visu.find(record);
ASSERT_NE(position, std::string::npos);
visu.replace(position, record.size(), "##$VisuCoreDim=" + dimension);
const auto path = WriteDataset("bruker_dimension_" + dimension, visu, 12);
ASSERT_FALSE(path.empty());
const std::string message = ReadImageInformationError(path);
ASSERT_FALSE(message.empty()) << "VisuCoreDim " << dimension << " was accepted";
EXPECT_NE(message.find("VisuCoreDim must be"), std::string::npos);
}
}

TEST(Bruker2dseqImageIO, ReadParaVision360Dataset)
{
const std::string path = WriteDataset("bruker2dseq_pv360", MakeVisuPars(), 12);
Expand Down
Loading