Skip to content

Commit

Permalink
Fix unit test failures.
Browse files Browse the repository at this point in the history
Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>
  • Loading branch information
imikejackson committed Apr 2, 2024
1 parent 5b5c738 commit 7d31b11
Show file tree
Hide file tree
Showing 7 changed files with 51 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ filter is provided with some specialized filters which implement different proje

This class was contributed to the Insight Journal by Gaetan Lehmann. The original paper can be found at <https://www.insight-journal.org/browse/publication/71>

## IMPORTANT NOTE

This filter will change the dimensionality of the Image Geometry that the data is tied to. This has the side effect of also
changing **every** Data Array that is store in the same AttributeMatrix as the input and output data array.

### Author

Gaetan Lehmann. Biologie du Developpement et de la Reproduction, INRA de Jouy-en-Josas, France.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ Parameters ITKBinaryProjectionImage::parameters() const
params.insertSeparator(Parameters::Separator{"Input Parameters"});
params.insert(std::make_unique<UInt32Parameter>(k_ProjectionDimension_Key, "Projection Dimension", "The dimension index to project. 0=Slowest moving dimension.", 0u));
params.insert(std::make_unique<Float64Parameter>(
k_ForegroundValue_Key, "ForegroundValue",
k_ForegroundValue_Key, "Foreground Value",
"Set the value in the image to consider as 'foreground'. Defaults to maximum value of PixelType. Subclasses may alias this to DilateValue or ErodeValue.", 1.0));
params.insert(std::make_unique<Float64Parameter>(k_BackgroundValue_Key, "Background Value",
"Set the value used as 'background'. Any pixel value which is not DilateValue is considered background. BackgroundValue is used for defining "
Expand Down Expand Up @@ -137,7 +137,24 @@ Result<> ITKBinaryProjectionImage::executeImpl(DataStructure& dataStructure, con
auto& imageGeom = dataStructure.getDataRefAs<ImageGeom>(imageGeomPath);
imageGeom.getLinkedGeometryData().addCellData(outputArrayPath);

return ITK::Execute<cxITKBinaryProjectionImage::ArrayOptionsType>(dataStructure, selectedInputArray, imageGeomPath, outputArrayPath, itkFunctor, shouldCancel);
auto result = ITK::Execute<cxITKBinaryProjectionImage::ArrayOptionsType>(dataStructure, selectedInputArray, imageGeomPath, outputArrayPath, itkFunctor, shouldCancel);

IArray& iArrayRef = dataStructure.getDataRefAs<IArray>(outputArrayPath);
auto iArrayTupleShape = iArrayRef.getTupleShape();
std::cout << fmt::format("{}", fmt::join(iArrayRef.getTupleShape(), ",")) << std::endl;

// Update the Image Geometry with the new dimensions
imageGeom.setDimensions({iArrayTupleShape[2], iArrayTupleShape[1], iArrayTupleShape[0]});

// Update the AttributeMatrix with the new tuple shape. THIS WILL ALSO CHANGE ANY OTHER DATA ARRAY THAT IS ALSO
// STORED IN THAT ATTRIBUTE MATRIX
auto amPathVector = outputArrayPath.getPathVector();
amPathVector.pop_back();
DataPath amPath(amPathVector);
AttributeMatrix& attributeMatrix = dataStructure.getDataRefAs<AttributeMatrix>(amPath);
attributeMatrix.resizeTuples(iArrayTupleShape);

return result;
}

namespace
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,9 +189,11 @@ Result<> CreateFeatureArrayFromElementArray::executeImpl(DataStructure& dataStru
// Resize the created array to the proper size
usize featureIdsMaxIdx = std::distance(featureIds.begin(), std::max_element(featureIds.cbegin(), featureIds.cend()));
usize maxValue = featureIds[featureIdsMaxIdx];
auto& cellFeatureAttrMat = dataStructure.getDataRefAs<AttributeMatrix>(pCellFeatureAttributeMatrixPathValue);

auto& createdArrayStore = createdArray.getIDataStoreRefAs<IDataStore>();
createdArrayStore.resizeTuples(std::vector<usize>{maxValue + 1});
cellFeatureAttrMat.resizeTuples(std::vector<usize>{maxValue + 1});

return ExecuteDataFunction(CopyCellDataFunctor{}, selectedCellArray.getDataType(), dataStructure, pSelectedCellArrayPathValue, pFeatureIdsArrayPathValue, createdArrayPath, shouldCancel);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,9 @@ void testElementArray(const DataPath& cellDataPath)
DataStructure dataStructure = UnitTest::LoadDataStructure(baseDataFilePath);
DataPath smallIn100Group({nx::core::Constants::k_DataContainer});

// This section creates the needed AttributeMatrix of size 1. The filter should be resizing as needed.
{
auto* computedFeatureData = AttributeMatrix::Create(dataStructure, k_Computed_CellData, std::vector<usize>{81}, dataStructure.getId(smallIn100Group));
AttributeMatrix::Create(dataStructure, k_Computed_CellData, std::vector<usize>{1}, dataStructure.getId(smallIn100Group));
}

DataPath featureIdsDataPath = smallIn100Group.createChildPath(nx::core::Constants::k_CellData).createChildPath(k_FeatureIDs);
Expand Down
14 changes: 9 additions & 5 deletions src/simplnx/DataStructure/DataStructure.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1040,13 +1040,17 @@ Result<> DataStructure::validateAttributeMatrices() const
Result<> result;
for(const auto& dataObject : m_DataObjects)
{
auto dataObjectType = dataObject.second.lock()->getDataObjectType();
if(dataObjectType == DataObject::Type::AttributeMatrix)
auto dataObjectSharedPtr = dataObject.second.lock();
if(dataObjectSharedPtr.get() != nullptr)
{
auto* attrMatPtr = dynamic_cast<AttributeMatrix*>(dataObject.second.lock().get());
if(nullptr != attrMatPtr)
auto dataObjectType = dataObjectSharedPtr->getDataObjectType();
if(dataObjectType == DataObject::Type::AttributeMatrix)
{
result = MergeResults(attrMatPtr->validate(), result);
auto* attrMatPtr = dynamic_cast<AttributeMatrix*>(dataObject.second.lock().get());
if(nullptr != attrMatPtr)
{
result = MergeResults(attrMatPtr->validate(), result);
}
}
}
}
Expand Down
12 changes: 3 additions & 9 deletions src/simplnx/Filter/IFilter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -225,15 +225,9 @@ IFilter::ExecuteResult IFilter::execute(DataStructure& dataStructure, const Argu
return {MakeErrorResult(-1, "Filter cancelled")};
}

Result<> validGeometries = MergeResults(dataStructure.validateGeometries(), executeImplResult);

Result<> validAttributeMatrices = dataStructure.validateAttributeMatrices();
if(validAttributeMatrices.invalid())
{
return ExecuteResult{std::move(validAttributeMatrices), std::move(preflightResult.outputValues) };
}

Result<> preflightActionsExecuteResult = MergeResults(std::move(preflightActionsResult), std::move(validGeometries));
Result<> validGeometryAndAttributeMatrices = MergeResults(dataStructure.validateGeometries(), dataStructure.validateAttributeMatrices());
validGeometryAndAttributeMatrices = MergeResults(validGeometryAndAttributeMatrices, executeImplResult);
Result<> preflightActionsExecuteResult = MergeResults(std::move(preflightActionsResult), std::move(validGeometryAndAttributeMatrices));

if(preflightActionsExecuteResult.invalid())
{
Expand Down
17 changes: 11 additions & 6 deletions test/UnitTestCommon/include/simplnx/UnitTest/UnitTestCommon.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -234,8 +234,13 @@ class TestFileSentinel
, m_InputArchiveName(std::move(inputArchiveName))
, m_ExpectedTopLevelOutput(std::move(expectedTopLevelOutput))
{
const auto result = decompress();
REQUIRE(result);
const auto errorCode = decompress();
if(errorCode)
{
std::cout << "std::error_code.value(): " << errorCode.value() << std::endl;
std::cout << "std::error_code.message(): " << errorCode.message() << std::endl;
REQUIRE(errorCode.value() == 0);
}
}

~TestFileSentinel()
Expand All @@ -257,18 +262,18 @@ class TestFileSentinel
* @brief Does the actual decompression of the archive.
* @return
*/
bool decompress()
std::error_code decompress()
{
reproc::options options;
options.redirect.parent = true;
options.deadline = reproc::milliseconds(600000);
options.working_directory = m_TestFilesDir.c_str();
options.nonblocking = false;

std::vector<std::string> args = {m_CMakeExecutable, "-E", "tar", "xvzf", fmt::format("{}/{}", m_TestFilesDir, m_InputArchiveName)};

auto&& [status, ec] = reproc::run(args, options);

return !ec;
auto resultPair = reproc::run(args, options);
return resultPair.second;
}

private:
Expand Down

0 comments on commit 7d31b11

Please sign in to comment.