Skip to content

Commit

Permalink
ENH: Filter Parameter for COPY/MOVE options are consistent (#983)
Browse files Browse the repository at this point in the history 
Created Enums that should be used for Array handling in these kinds of filters
Update the python bindings to also expose the enums to the python side.

Signed-off-by: Michael Jackson <[email protected]>
  • Loading branch information
@imikejackson
imikejackson authored Jun 3, 2024
1 parent b33bc87 commit 825143f
Show file tree
Hide file tree
Showing 10 changed files with 64 additions and 79 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
12 changes: 7 additions & 5 deletions src/Plugins/SimplnxCore/src/SimplnxCore/Filters/CopyDataObjectFilter.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,15 +51,17 @@ Parameters CopyDataObjectFilter::parameters() const
{
Parameters params;

params.insertSeparator(Parameters::Separator{"Input Parameter(s)"});
params.insertLinkableParameter(std::make_unique<BoolParameter>(k_UseNewParent_Key, "Copy to New Parent", "Copy all the DataObjects to a different Group", false));
params.insert(std::make_unique<DataGroupSelectionParameter>(k_NewPath_Key, "New Parent Destination", "DataPath to parent Group in which to store the copied DataObject(s)", DataPath{},
BaseGroup::GetAllGroupTypes()));
params.insert(std::make_unique<StringParameter>(k_NewPathSuffix_Key, "Copied Object(s) Suffix", "Suffix string to be appended to each copied DataObject. Can be blank.", "_COPY"));

params.insertSeparator(Parameters::Separator{"Input Data Objects"});
params.insert(std::make_unique<MultiPathSelectionParameter>(k_DataPath_Key, "Objects to copy", "A list of DataPaths to the DataObjects to be copied", MultiPathSelectionParameter::ValueType{}));

params.insertLinkableParameter(std::make_unique<BoolParameter>(k_UseNewParent_Key, "Copy to New Parent", "Copy all the DataObjects to a new BaseGroup", false));
params.insert(std::make_unique<DataGroupSelectionParameter>(k_NewPath_Key, "Copied Parent Group", "DataPath to parent BaseGroup in which to store the copied DataObject(s)", DataPath{},
BaseGroup::GetAllGroupTypes()));
params.insert(std::make_unique<StringParameter>(k_NewPathSuffix_Key, "Copied Object(s) Suffix", "Suffix string to be appended to each copied DataObject", "_COPY"));

params.linkParameters(k_UseNewParent_Key, k_NewPath_Key, true);

return params;
}

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
Loading

0 comments on commit 825143f

Please sign in to comment.