Skip to content

Commit

Permalink
ENH: Standardize on ArrayHandling for filters.
Browse files Browse the repository at this point in the history 
- There was a miss-mash of "Copy", "Move" and "Move", "Copy"
- Only Copy and Move was exposed to the Python bindings

Signed-off-by: Michael Jackson <[email protected]>
  • Loading branch information
@imikejackson
imikejackson committed Jun 3, 2024
1 parent 9869b72 commit 154ed61
Show file tree
Hide file tree
Showing 9 changed files with 57 additions and 74 deletions.
4 changes: 2 additions & 2 deletions src/Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/ExtractVertexGeometry.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ Result<> ExtractVertexGeometry::operator()()
}

// If we are copying arrays, either with or without a mask, this code is applicable.
if(m_InputValues->ArrayHandling == static_cast<ChoicesParameter::ValueType>(ArrayHandlingType::CopyArrays))
if(m_InputValues->ArrayHandling == to_underlying(ArrayHandlingType::Copy))
{
// Since we made copies of the DataArrays, we can safely resize the entire Attribute Matrix,
// which will resize all the contained DataArrays
Expand All @@ -168,7 +168,7 @@ Result<> ExtractVertexGeometry::operator()()
// took care of renaming/moving the arrays for us and we are done.

// If we are MOVING arrays AND we are using a mask then we need this code block to execute
if(m_InputValues->ArrayHandling == static_cast<ChoicesParameter::ValueType>(ArrayHandlingType::MoveArrays) && m_InputValues->UseMask && vertexCount != cellCount)
if(m_InputValues->ArrayHandling == to_underlying(ArrayHandlingType::Move) && m_InputValues->UseMask && vertexCount != cellCount)
{
// The arrays have already been moved at this point, so the source and
// destinations are the same. This should work.
Expand Down
6 changes: 0 additions & 6 deletions src/Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/ExtractVertexGeometry.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,6 @@ class SIMPLNXCORE_EXPORT ExtractVertexGeometry
ExtractVertexGeometry& operator=(const ExtractVertexGeometry&) = delete;
ExtractVertexGeometry& operator=(ExtractVertexGeometry&&) noexcept = delete;

enum class ArrayHandlingType : ChoicesParameter::ValueType
{
MoveArrays,
CopyArrays
};

Result<> operator()();

const std::atomic_bool& getCancel();
Expand Down
33 changes: 11 additions & 22 deletions src/Plugins/SimplnxCore/src/SimplnxCore/Filters/CreateGeometryFilter.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,7 @@ Result<> checkGeometryArraysCompatible(const Float32Array& vertices, const UInt6
uint64 idx = 0;
for(usize i = 0; i < cells.getSize(); i++)
{
if(cells[i] > idx)
{
idx = cells[i];
}
idx = std::max(cells[i], idx);
}
if((idx + 1) > numVertices)
{
Expand Down Expand Up @@ -120,8 +117,8 @@ Parameters CreateGeometryFilter::parameters() const
IGeometry::GetAllLengthUnitStrings()));
params.insert(std::make_unique<BoolParameter>(k_WarningsAsErrors_Key, "Treat Geometry Warnings as Errors", "Whether run time warnings for Geometries should be treated as errors", false));
params.insert(std::make_unique<ChoicesParameter>(k_ArrayHandling_Key, "Array Handling",
"Determines if the arrays that make up the geometry primitives should be Moved or Copied to the created Geometry object.", 0,
ChoicesParameter::Choices{"Copy Array", "Move Array" /*, "Reference Array"*/}));
"Determines if the arrays that make up the geometry primitives should be Moved or Copied to the created Geometry object.",
to_underlying(ArrayHandlingType::Move), ChoicesParameter::Choices{"Copy Attribute Arrays", "Move Attribute Arrays" /*, "Reference Array"*/}));

params.insert(std::make_unique<VectorUInt64Parameter>(k_Dimensions_Key, "Dimensions", "The number of cells in each of the X, Y, Z directions", std::vector<uint64_t>{20ULL, 60ULL, 200ULL},
std::vector<std::string>{"X"s, "Y"s, "Z"s}));
Expand Down Expand Up @@ -216,9 +213,7 @@ IFilter::PreflightResult CreateGeometryFilter::preflightImpl(const DataStructure
{
auto pGeometryPath = filterArgs.value<DataPath>(k_GeometryPath_Key);
auto pGeometryType = filterArgs.value<ChoicesParameter::ValueType>(k_GeometryType_Key);
auto pWarningsAsErrors = filterArgs.value<bool>(k_WarningsAsErrors_Key);
auto pArrayHandling = filterArgs.value<ChoicesParameter::ValueType>(k_ArrayHandling_Key);
auto pMoveArrays = pArrayHandling == k_MoveArray;

nx::core::Result<OutputActions> resultOutputActions;
std::vector<PreflightValue> preflightUpdatedValues;
Expand Down Expand Up @@ -305,13 +300,12 @@ IFilter::PreflightResult CreateGeometryFilter::preflightImpl(const DataStructure
xBounds->getNumberOfTuples(), yBounds->getNumberOfTuples(), zBounds->getNumberOfTuples())}})};
}

auto createRectGridGeometryAction =
std::make_unique<CreateRectGridGeometryAction>(pGeometryPath, pXBoundsPath, pYBoundsPath, pZBoundsPath, pCellAMName, IDataCreationAction::ArrayHandlingType{pArrayHandling});
auto createRectGridGeometryAction = std::make_unique<CreateRectGridGeometryAction>(pGeometryPath, pXBoundsPath, pYBoundsPath, pZBoundsPath, pCellAMName, ArrayHandlingType{pArrayHandling});
resultOutputActions.value().appendAction(std::move(createRectGridGeometryAction));
}
if(pGeometryType == k_VertexGeometry) // VertexGeom
{
auto createVertexGeomAction = std::make_unique<CreateVertexGeometryAction>(pGeometryPath, pVertexListPath, pVertexAMName, IDataCreationAction::ArrayHandlingType{pArrayHandling});
auto createVertexGeomAction = std::make_unique<CreateVertexGeometryAction>(pGeometryPath, pVertexListPath, pVertexAMName, ArrayHandlingType{pArrayHandling});
resultOutputActions.value().appendAction(std::move(createVertexGeomAction));
}
if(pGeometryType == k_EdgeGeometry) // EdgeGeom
Expand All @@ -323,8 +317,7 @@ IFilter::PreflightResult CreateGeometryFilter::preflightImpl(const DataStructure
return {nonstd::make_unexpected(std::vector<Error>{Error{-9845, fmt::format("Cannot find selected edge list at path '{}'", pEdgeListPath.toString())}})};
}

auto createEdgeGeomAction =
std::make_unique<CreateEdgeGeometryAction>(pGeometryPath, pVertexListPath, pEdgeListPath, pVertexAMName, pEdgeAMName, IDataCreationAction::ArrayHandlingType{pArrayHandling});
auto createEdgeGeomAction = std::make_unique<CreateEdgeGeometryAction>(pGeometryPath, pVertexListPath, pEdgeListPath, pVertexAMName, pEdgeAMName, ArrayHandlingType{pArrayHandling});
resultOutputActions.value().appendAction(std::move(createEdgeGeomAction));
}
if(pGeometryType == k_TriangleGeometry) // TriangleGeom
Expand All @@ -335,8 +328,7 @@ IFilter::PreflightResult CreateGeometryFilter::preflightImpl(const DataStructure
return {nonstd::make_unexpected(std::vector<Error>{Error{-9846, fmt::format("Cannot find selected triangle list at path '{}'", pTriangleListPath.toString())}})};
}

auto createTriangleGeomAction =
std::make_unique<CreateTriangleGeometryAction>(pGeometryPath, pVertexListPath, pTriangleListPath, pVertexAMName, pFaceAMName, IDataCreationAction::ArrayHandlingType{pArrayHandling});
auto createTriangleGeomAction = std::make_unique<CreateTriangleGeometryAction>(pGeometryPath, pVertexListPath, pTriangleListPath, pVertexAMName, pFaceAMName, ArrayHandlingType{pArrayHandling});
resultOutputActions.value().appendAction(std::move(createTriangleGeomAction));
}
if(pGeometryType == k_QuadGeometry) // QuadGeom
Expand All @@ -347,8 +339,7 @@ IFilter::PreflightResult CreateGeometryFilter::preflightImpl(const DataStructure
return {nonstd::make_unexpected(std::vector<Error>{Error{-9847, fmt::format("Cannot find selected quadrilateral list at path '{}'", pQuadListPath.toString())}})};
}

auto createQuadGeomAction =
std::make_unique<CreateQuadGeometryAction>(pGeometryPath, pVertexListPath, pQuadListPath, pVertexAMName, pFaceAMName, IDataCreationAction::ArrayHandlingType{pArrayHandling});
auto createQuadGeomAction = std::make_unique<CreateQuadGeometryAction>(pGeometryPath, pVertexListPath, pQuadListPath, pVertexAMName, pFaceAMName, ArrayHandlingType{pArrayHandling});
resultOutputActions.value().appendAction(std::move(createQuadGeomAction));
}
if(pGeometryType == k_TetGeometry) // TetrahedralGeom
Expand All @@ -359,8 +350,7 @@ IFilter::PreflightResult CreateGeometryFilter::preflightImpl(const DataStructure
return {nonstd::make_unexpected(std::vector<Error>{Error{-9848, fmt::format("Cannot find selected quadrilateral list at path '{}'", pTetListPath.toString())}})};
}

auto createTetGeomAction =
std::make_unique<CreateTetrahedralGeometryAction>(pGeometryPath, pVertexListPath, pTetListPath, pVertexAMName, pCellAMName, IDataCreationAction::ArrayHandlingType{pArrayHandling});
auto createTetGeomAction = std::make_unique<CreateTetrahedralGeometryAction>(pGeometryPath, pVertexListPath, pTetListPath, pVertexAMName, pCellAMName, ArrayHandlingType{pArrayHandling});
resultOutputActions.value().appendAction(std::move(createTetGeomAction));
}
if(pGeometryType == k_HexGeometry) // HexahedralGeom
Expand All @@ -371,8 +361,7 @@ IFilter::PreflightResult CreateGeometryFilter::preflightImpl(const DataStructure
return {nonstd::make_unexpected(std::vector<Error>{Error{-9849, fmt::format("Cannot find selected quadrilateral list at path '{}'", pHexListPath.toString())}})};
}

auto createHexGeomAction =
std::make_unique<CreateHexahedralGeometryAction>(pGeometryPath, pVertexListPath, pHexListPath, pVertexAMName, pCellAMName, IDataCreationAction::ArrayHandlingType{pArrayHandling});
auto createHexGeomAction = std::make_unique<CreateHexahedralGeometryAction>(pGeometryPath, pVertexListPath, pHexListPath, pVertexAMName, pCellAMName, ArrayHandlingType{pArrayHandling});
resultOutputActions.value().appendAction(std::move(createHexGeomAction));
}

Expand All @@ -387,7 +376,7 @@ Result<> CreateGeometryFilter::executeImpl(DataStructure& dataStructure, const A
auto geometryPath = filterArgs.value<DataPath>(k_GeometryPath_Key);
auto geometryType = filterArgs.value<ChoicesParameter::ValueType>(k_GeometryType_Key);
auto treatWarningsAsErrors = filterArgs.value<bool>(k_WarningsAsErrors_Key);
auto moveArrays = filterArgs.value<ChoicesParameter::ValueType>(k_ArrayHandling_Key) == k_MoveArray;
// auto moveArrays = filterArgs.value<ChoicesParameter::ValueType>(k_ArrayHandling_Key) == k_MoveArray;

auto iGeometry = dataStructure.getDataAs<IGeometry>(geometryPath);
auto lengthUnit = static_cast<IGeometry::LengthUnit>(filterArgs.value<ChoicesParameter::ValueType>(k_LengthUnitType_Key));
Expand Down
10 changes: 4 additions & 6 deletions src/Plugins/SimplnxCore/src/SimplnxCore/Filters/ExtractVertexGeometryFilter.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@ Parameters ExtractVertexGeometryFilter::parameters() const
Parameters params;
// Create the parameter descriptors that are needed for this filter
params.insertSeparator(Parameters::Separator{"Input Parameter(s)"});
params.insert(std::make_unique<ChoicesParameter>(k_ArrayHandling_Key, "Array Handling", "[0] Move or [1] Copy input data arrays", 0,
ChoicesParameter::Choices{"Move Attribute Arrays", "Copy Attribute Arrays"}));
params.insert(std::make_unique<ChoicesParameter>(k_ArrayHandling_Key, "Array Handling", "[0] Move or [1] Copy input data arrays", to_underlying(ArrayHandlingType::Move),
ChoicesParameter::Choices{"Copy Attribute Arrays", "Move Attribute Arrays"}));

params.insertSeparator(Parameters::Separator{"Optional Data Mask"});
params.insertLinkableParameter(std::make_unique<BoolParameter>(k_UseMask_Key, "Use Mask Array", "Specifies whether or not to use a mask array", false));
Expand Down Expand Up @@ -118,8 +118,6 @@ IFilter::PreflightResult ExtractVertexGeometryFilter::preflightImpl(const DataSt

nx::core::Result<OutputActions> resultOutputActions;

const ExtractVertexGeometry::ArrayHandlingType arrayHandlingType = static_cast<ExtractVertexGeometry::ArrayHandlingType>(pArrayHandlingValue);

const IGridGeometry& geometry = dataStructure.getDataRefAs<IGridGeometry>({pInputGeometryPathValue});
SizeVec3 dims = geometry.getDimensions();
usize geomElementCount = dims[0] * dims[1] * dims[2];
Expand Down Expand Up @@ -173,13 +171,13 @@ IFilter::PreflightResult ExtractVertexGeometryFilter::preflightImpl(const DataSt
{
const IDataArray& dataArray = dataStructure.getDataRefAs<IDataArray>(dataPath);

if(arrayHandlingType == ExtractVertexGeometry::ArrayHandlingType::CopyArrays)
if(pArrayHandlingValue == to_underlying(ArrayHandlingType::Copy))
{
DataPath newDataPath = vertexAttrMatrixPath.createChildPath(dataPath.getTargetName());
auto createArrayAction = std::make_unique<CreateArrayAction>(dataArray.getDataType(), dataArray.getTupleShape(), dataArray.getComponentShape(), newDataPath);
resultOutputActions.value().appendAction(std::move(createArrayAction));
}
else if(arrayHandlingType == ExtractVertexGeometry::ArrayHandlingType::MoveArrays)
else if(pArrayHandlingValue == to_underlying(ArrayHandlingType::Move))
{
auto moveDataAction = std::make_unique<MoveDataAction>(dataPath, vertexAttrMatrixPath);
resultOutputActions.value().appendAction(std::move(moveDataAction));
Expand Down
4 changes: 2 additions & 2 deletions src/Plugins/SimplnxCore/test/ExtractPipelineToFileTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ fs::path k_JsonOutputFile(k_OutputFileName.string() + Pipeline::k_SIMPLExtension
fs::path k_NXOutputFile(k_OutputFileName.string() + Pipeline::k_Extension.str());
} // namespace

TEST_CASE("SimplnxCore::ExtractPipelineToFileFilter : Valid Execution", "[SimplnxCore][ExtractPipelineToFileFilter]")
TEST_CASE("SimplnxCore::ExtractPipelineToFileFilter: Valid Execution", "[SimplnxCore][ExtractPipelineToFileFilter]")
{
// Instantiate the filter, a DataStructure object and an Arguments Object
DataStructure dataStructure;
Expand All @@ -49,7 +49,7 @@ TEST_CASE("SimplnxCore::ExtractPipelineToFileFilter : Valid Execution", "[Simpln
fs::remove(k_JsonOutputFile);
}

TEST_CASE("SimplnxCore::ExtractPipelineToFileFilter : Valid Execution - incorrect output extension", "[SimplnxCore][ExtractPipelineToFileFilter]")
TEST_CASE("SimplnxCore::ExtractPipelineToFileFilter: Valid Execution - incorrect output extension", "[SimplnxCore][ExtractPipelineToFileFilter]")
{
// Instantiate the filter, a DataStructure object and an Arguments Object
DataStructure dataStructure;
Expand Down
14 changes: 6 additions & 8 deletions src/Plugins/SimplnxCore/test/ExtractVertexGeometryTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@ const std::string k_WrongAttrMatName = "WrongAttrMatrix";
const std::string k_FloatArrayName = "FloatArray";
const std::string k_MaskArrayName = "MaskArray";
const DataPath k_VertexDataContainerPath = {{"VertexDataContainer"}};
const int32 k_MoveArrays = 0;
const int32 k_CopyArrays = 1;

namespace ExtractVertexGeometryTest
{
Expand Down Expand Up @@ -88,7 +86,7 @@ TEST_CASE("SimplnxCore::ExtractVertexGeometry: Data Array With Wrong Tuple Count
Arguments args;

// Create default Parameters for the filter.
args.insertOrAssign(ExtractVertexGeometryFilter::k_ArrayHandling_Key, std::make_any<ChoicesParameter::ValueType>(k_MoveArrays));
args.insertOrAssign(ExtractVertexGeometryFilter::k_ArrayHandling_Key, std::make_any<ChoicesParameter::ValueType>(to_underlying(ArrayHandlingType::Move)));
args.insertOrAssign(ExtractVertexGeometryFilter::k_InputGeometryPath_Key, std::make_any<DataPath>(DataPath{{k_ImageGeometryName}}));
args.insertOrAssign(ExtractVertexGeometryFilter::k_IncludedDataArrayPaths_Key,
std::make_any<MultiArraySelectionParameter::ValueType>(MultiArraySelectionParameter::ValueType{DataPath{{k_ImageGeometryName, k_WrongAttrMatName, k_FloatArrayName}}}));
Expand All @@ -108,7 +106,7 @@ TEST_CASE("SimplnxCore::ExtractVertexGeometry: Mask Array With Wrong Tuple Count
Arguments args;

// Create default Parameters for the filter.
args.insertOrAssign(ExtractVertexGeometryFilter::k_ArrayHandling_Key, std::make_any<ChoicesParameter::ValueType>(k_MoveArrays));
args.insertOrAssign(ExtractVertexGeometryFilter::k_ArrayHandling_Key, std::make_any<ChoicesParameter::ValueType>(to_underlying(ArrayHandlingType::Move)));
args.insertOrAssign(ExtractVertexGeometryFilter::k_InputGeometryPath_Key, std::make_any<DataPath>(DataPath{{k_ImageGeometryName}}));
args.insertOrAssign(ExtractVertexGeometryFilter::k_IncludedDataArrayPaths_Key,
std::make_any<MultiArraySelectionParameter::ValueType>(MultiArraySelectionParameter::ValueType{DataPath{{k_ImageGeometryName, k_CellAttrMatName, k_FloatArrayName}}}));
Expand All @@ -132,7 +130,7 @@ TEST_CASE("SimplnxCore::ExtractVertexGeometry: Move cell data arrays", "[Simplnx
MultiArraySelectionParameter::ValueType inputDataPaths = {DataPath({k_ImageGeometryName, k_CellAttrMatName, k_FloatArrayName})};

// Create default Parameters for the filter.
args.insertOrAssign(ExtractVertexGeometryFilter::k_ArrayHandling_Key, std::make_any<ChoicesParameter::ValueType>(k_MoveArrays));
args.insertOrAssign(ExtractVertexGeometryFilter::k_ArrayHandling_Key, std::make_any<ChoicesParameter::ValueType>(to_underlying(ArrayHandlingType::Move)));
args.insertOrAssign(ExtractVertexGeometryFilter::k_InputGeometryPath_Key, std::make_any<DataPath>(DataPath{{k_ImageGeometryName}}));
args.insertOrAssign(ExtractVertexGeometryFilter::k_IncludedDataArrayPaths_Key, std::make_any<MultiArraySelectionParameter::ValueType>(inputDataPaths));
args.insertOrAssign(ExtractVertexGeometryFilter::k_VertexGeometryPath_Key, std::make_any<DataPath>(DataPath{k_VertexDataContainerPath}));
Expand Down Expand Up @@ -162,7 +160,7 @@ TEST_CASE("SimplnxCore::ExtractVertexGeometry: Copy cell data arrays", "[Simplnx
MultiArraySelectionParameter::ValueType inputDataPaths = {DataPath({k_ImageGeometryName, k_CellAttrMatName, k_FloatArrayName})};

// Create default Parameters for the filter.
args.insertOrAssign(ExtractVertexGeometryFilter::k_ArrayHandling_Key, std::make_any<ChoicesParameter::ValueType>(k_CopyArrays));
args.insertOrAssign(ExtractVertexGeometryFilter::k_ArrayHandling_Key, std::make_any<ChoicesParameter::ValueType>(to_underlying(ArrayHandlingType::Copy)));
args.insertOrAssign(ExtractVertexGeometryFilter::k_InputGeometryPath_Key, std::make_any<DataPath>(DataPath{{k_ImageGeometryName}}));
args.insertOrAssign(ExtractVertexGeometryFilter::k_IncludedDataArrayPaths_Key, std::make_any<MultiArraySelectionParameter::ValueType>(inputDataPaths));
args.insertOrAssign(ExtractVertexGeometryFilter::k_VertexGeometryPath_Key, std::make_any<DataPath>(DataPath{k_VertexDataContainerPath}));
Expand Down Expand Up @@ -205,7 +203,7 @@ TEST_CASE("SimplnxCore::ExtractVertexGeometry: Move cell data arrays with mask",
MultiArraySelectionParameter::ValueType inputDataPaths = {DataPath({k_ImageGeometryName, k_CellAttrMatName, k_FloatArrayName})};

// Create default Parameters for the filter.
args.insertOrAssign(ExtractVertexGeometryFilter::k_ArrayHandling_Key, std::make_any<ChoicesParameter::ValueType>(k_MoveArrays));
args.insertOrAssign(ExtractVertexGeometryFilter::k_ArrayHandling_Key, std::make_any<ChoicesParameter::ValueType>(to_underlying(ArrayHandlingType::Move)));
args.insertOrAssign(ExtractVertexGeometryFilter::k_InputGeometryPath_Key, std::make_any<DataPath>(DataPath{{k_ImageGeometryName}}));
args.insertOrAssign(ExtractVertexGeometryFilter::k_IncludedDataArrayPaths_Key, std::make_any<MultiArraySelectionParameter::ValueType>(inputDataPaths));
args.insertOrAssign(ExtractVertexGeometryFilter::k_VertexGeometryPath_Key, std::make_any<DataPath>(DataPath{k_VertexDataContainerPath}));
Expand Down Expand Up @@ -238,7 +236,7 @@ TEST_CASE("SimplnxCore::ExtractVertexGeometry: Copy cell data arrays with mask",
MultiArraySelectionParameter::ValueType inputDataPaths = {DataPath({k_ImageGeometryName, k_CellAttrMatName, k_FloatArrayName})};

// Create default Parameters for the filter.
args.insertOrAssign(ExtractVertexGeometryFilter::k_ArrayHandling_Key, std::make_any<ChoicesParameter::ValueType>(k_CopyArrays));
args.insertOrAssign(ExtractVertexGeometryFilter::k_ArrayHandling_Key, std::make_any<ChoicesParameter::ValueType>(to_underlying(ArrayHandlingType::Copy)));
args.insertOrAssign(ExtractVertexGeometryFilter::k_InputGeometryPath_Key, std::make_any<DataPath>(DataPath{{k_ImageGeometryName}}));
args.insertOrAssign(ExtractVertexGeometryFilter::k_IncludedDataArrayPaths_Key, std::make_any<MultiArraySelectionParameter::ValueType>(inputDataPaths));
args.insertOrAssign(ExtractVertexGeometryFilter::k_VertexGeometryPath_Key, std::make_any<DataPath>(DataPath{k_VertexDataContainerPath}));
Expand Down
Loading

0 comments on commit 154ed61

Please sign in to comment.