Skip to content

Commit

Permalink
BUG FIX: Data object parent ids are now renumbered correctly when obj…
Browse files Browse the repository at this point in the history
…ect id clashes are found. (#1102)

* Fixes a bug where if a DREAM3D file is read into a data structure that already has existing data objects in it and the file is read in without specifying data paths, then an infinite loop occurs when getting the data paths of each object. This ONLY happens when using Python or executing non-GUI C++ code because the GUI never leaves the data paths variable empty. This is because the data object parent ids were getting renumbered incorrectly.

* Added an additional unit test case to DREAM3DFileTest to test that this is working correctly.
  • Loading branch information
joeykleingers authored Oct 10, 2024
1 parent 51372c2 commit 8efcc3a
Show file tree
Hide file tree
Showing 22 changed files with 116 additions and 59 deletions.
53 changes: 53 additions & 0 deletions src/Plugins/SimplnxCore/test/DREAM3DFileTest.cpp
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
#include "SimplnxCore/Filters/CreateDataArrayFilter.hpp"
#include "SimplnxCore/Filters/CreateImageGeometryFilter.hpp"
#include "SimplnxCore/Filters/ReadDREAM3DFilter.hpp"
#include "SimplnxCore/SimplnxCore_test_dirs.hpp"

#include "simplnx/Core/Application.hpp"
Expand Down Expand Up @@ -362,3 +365,53 @@ TEST_CASE("DREAM3DFileTest:Import/Export Multi-DREAM3D Filter Test")
REQUIRE(importDataStructure.getData(DataPath({DataNames::k_Group1Name})) != nullptr);
REQUIRE(importDataStructure.getData(DataPath({DataNames::k_Group2Name})) != nullptr);
}

TEST_CASE("DREAM3DFileTest: Existing Data Objects Test")
{
DataStructure ds;
{
CreateImageGeometryFilter filter;
Arguments args;
args.insert(CreateImageGeometryFilter::k_GeometryDataPath_Key, std::make_any<DataPath>(DataPath({"New Geometry"})));
args.insert(CreateImageGeometryFilter::k_CellDataName_Key, std::make_any<std::string>("Cell Data"));
args.insert(CreateImageGeometryFilter::k_Dimensions_Key, std::make_any<std::vector<uint64_t>>(std::vector<uint64_t>{480, 640, 1}));
args.insert(CreateImageGeometryFilter::k_Origin_Key, std::make_any<std::vector<float32>>(std::vector<float32>{0, 0, 0}));
args.insert(CreateImageGeometryFilter::k_Spacing_Key, std::make_any<std::vector<float32>>(std::vector<float32>{0.5, 0.5, 0.12}));
auto executeResult = filter.execute(ds, args);
REQUIRE(executeResult.result.valid());
}

{
CreateDataArrayFilter filter;
Arguments args;
args.insert(CreateDataArrayFilter::k_NumericType_Key, std::make_any<NumericType>(NumericType::float32));
args.insert(CreateDataArrayFilter::k_NumComps_Key, std::make_any<uint64>(1));
args.insert(CreateDataArrayFilter::k_DataPath_Key, std::make_any<DataPath>(DataPath({"New Geometry", "Cell Data", "Array 1"})));
args.insert(CreateDataArrayFilter::k_InitializationValue_Key, std::make_any<std::string>("0"));
auto executeResult = filter.execute(ds, args);
REQUIRE(executeResult.result.valid());
}

{
CreateDataArrayFilter filter;
Arguments args;
args.insert(CreateDataArrayFilter::k_NumericType_Key, std::make_any<NumericType>(NumericType::float32));
args.insert(CreateDataArrayFilter::k_NumComps_Key, std::make_any<uint64>(1));
args.insert(CreateDataArrayFilter::k_DataPath_Key, std::make_any<DataPath>(DataPath({"New Geometry", "Cell Data", "Array 2"})));
args.insert(CreateDataArrayFilter::k_InitializationValue_Key, std::make_any<std::string>("0"));
auto executeResult = filter.execute(ds, args);
REQUIRE(executeResult.result.valid());
}

{
const nx::core::UnitTest::TestFileSentinel testDataSentinel(nx::core::unit_test::k_CMakeExecutable, nx::core::unit_test::k_TestFilesDir, "Small_IN100_dream3d_v2.tar.gz", "Small_IN100.dream3d");

ReadDREAM3DFilter filter;
Arguments args;
Dream3dImportParameter::ImportData importData;
importData.FilePath = fs::path(fmt::format("{}/Small_IN100.dream3d", unit_test::k_TestFilesDir));
args.insert(ReadDREAM3DFilter::k_ImportFileData, importData);
auto executeResult = filter.execute(ds, args);
REQUIRE(executeResult.result.valid());
}
}
4 changes: 2 additions & 2 deletions src/simplnx/DataStructure/BaseGroup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -247,9 +247,9 @@ std::vector<std::string> BaseGroup::GetChildrenNames()
return m_DataMap.getNames();
}

void BaseGroup::checkUpdatedIdsImpl(const std::vector<std::pair<IdType, IdType>>& updatedIds)
void BaseGroup::checkUpdatedIdsImpl(const std::unordered_map<DataObject::IdType, DataObject::IdType>& updatedIdsMap)
{
m_DataMap.updateIds(updatedIds);
m_DataMap.updateIds(updatedIdsMap);
}

std::vector<DataObject::IdType> BaseGroup::GetChildrenIds()
Expand Down
4 changes: 2 additions & 2 deletions src/simplnx/DataStructure/BaseGroup.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -357,9 +357,9 @@ class SIMPLNX_EXPORT BaseGroup : public DataObject

/**
* @brief Updates the DataMap IDs. Should only be called by DataObject::checkUpdatedIds.
* @param updatedIds
* @param updatedIdsMap
*/
void checkUpdatedIdsImpl(const std::vector<std::pair<IdType, IdType>>& updatedIds) override;
void checkUpdatedIdsImpl(const std::unordered_map<DataObject::IdType, DataObject::IdType>& updatedIdsMap) override;

/**
* @brief Checks if the provided DataObject can be added to the container.
Expand Down
2 changes: 1 addition & 1 deletion src/simplnx/DataStructure/DataMap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ DataMap& DataMap::operator=(DataMap&& rhs) noexcept
return *this;
}

void DataMap::updateIds(const std::vector<std::pair<IdType, IdType>>& updatedIds)
void DataMap::updateIds(const std::unordered_map<DataObject::IdType, DataObject::IdType>& updatedIds)
{
using UpdatedValueType = std::pair<IdType, std::shared_ptr<DataObject>>;
std::list<UpdatedValueType> movedValues;
Expand Down
7 changes: 4 additions & 3 deletions src/simplnx/DataStructure/DataMap.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include <memory>
#include <optional>
#include <string>
#include <unordered_map>
#include <vector>

namespace nx::core
Expand Down Expand Up @@ -285,10 +286,10 @@ class SIMPLNX_EXPORT DataMap
DataMap& operator=(DataMap&& rhs) noexcept;

/**
* @brief Updates the map IDs using a vector of updated IDs and their new values.
* @param updatedIds
* @brief Updates the map IDs using an unordered map of IDs and their new values.
* @param updatedIdsMap
*/
void updateIds(const std::vector<std::pair<IdType, IdType>>& updatedIds);
void updateIds(const std::unordered_map<IdType, IdType>& updatedIdsMap);

private:
MapType m_Map;
Expand Down
19 changes: 11 additions & 8 deletions src/simplnx/DataStructure/DataObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -101,19 +101,22 @@ void DataObject::setId(IdType newId)
m_Id = newId;
}

void DataObject::checkUpdatedIds(const std::vector<std::pair<IdType, IdType>>& updatedIds)
void DataObject::checkUpdatedIds(const std::unordered_map<DataObject::IdType, DataObject::IdType>& updatedIdsMap)
{
for(const auto& updatedId : updatedIds)
{
// Update parent list
std::replace(m_ParentList.begin(), m_ParentList.end(), updatedId.first, updatedId.second);
}
// Use std::transform to map IDs
ParentCollectionType newParentList;
std::transform(m_ParentList.begin(), m_ParentList.end(), std::back_inserter(newParentList), [&updatedIdsMap](uint64 id) -> uint64 {
auto it = updatedIdsMap.find(id);
return (it != updatedIdsMap.end()) ? it->second : id;
});

m_ParentList = newParentList;

// For derived classes
checkUpdatedIdsImpl(updatedIds);
checkUpdatedIdsImpl(updatedIdsMap);
}

void DataObject::checkUpdatedIdsImpl(const std::vector<std::pair<IdType, IdType>>& updatedIds)
void DataObject::checkUpdatedIdsImpl(const std::unordered_map<DataObject::IdType, DataObject::IdType>& updatedIdsMap)
{
}

Expand Down
7 changes: 4 additions & 3 deletions src/simplnx/DataStructure/DataObject.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include <memory>
#include <optional>
#include <string>
#include <unordered_map>
#include <vector>

namespace nx::core
Expand Down Expand Up @@ -288,15 +289,15 @@ class SIMPLNX_EXPORT DataObject

/**
* @brief Notifies the DataObject of DataObject IDs that have been changed by the DataStructure.
* @param updatedIds std::pair ordered as {old ID, new ID}
* @param updatedIdsMap std::unordered_map containing the mappings between the old IDs and the new IDs
*/
void checkUpdatedIds(const std::vector<std::pair<IdType, IdType>>& updatedIds);
void checkUpdatedIds(const std::unordered_map<DataObject::IdType, DataObject::IdType>& updatedIdsMap);

/**
* @brief Calls specialized checks for derived classes. Should only be called by checkUpdatedIds.
* @param updatedIds
*/
virtual void checkUpdatedIdsImpl(const std::vector<std::pair<IdType, IdType>>& updatedIds);
virtual void checkUpdatedIdsImpl(const std::unordered_map<DataObject::IdType, DataObject::IdType>& updatedIdsMap);

/**
* @brief Attempts to add the specified DataObject to the target DataStructure.
Expand Down
9 changes: 4 additions & 5 deletions src/simplnx/DataStructure/DataStructure.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -831,8 +831,7 @@ void DataStructure::resetIds(DataObject::IdType startingId)

// Update DataObject IDs and track changes
WeakCollectionType newCollection;
using UpdatedId = std::pair<DataObject::IdType, DataObject::IdType>;
std::vector<UpdatedId> updatedIds;
std::unordered_map<DataObject::IdType, DataObject::IdType> updatedIdsMap;
for(auto& dataObjectIter : m_DataObjects)
{
auto dataObjectPtr = dataObjectIter.second.lock();
Expand All @@ -845,7 +844,7 @@ void DataStructure::resetIds(DataObject::IdType startingId)
auto newId = generateId();

dataObjectPtr->setId(newId);
updatedIds.push_back({oldId, newId});
updatedIdsMap[oldId] = newId;

newCollection.insert({newId, dataObjectPtr});
}
Expand All @@ -859,10 +858,10 @@ void DataStructure::resetIds(DataObject::IdType startingId)
auto dataObjectPtr = dataObjectIter.second.lock();
if(dataObjectPtr != nullptr)
{
dataObjectPtr->checkUpdatedIds(updatedIds);
dataObjectPtr->checkUpdatedIds(updatedIdsMap);
}
}
m_RootGroup.updateIds(updatedIds);
m_RootGroup.updateIds(updatedIdsMap);
}

void DataStructure::exportHierarchyAsGraphViz(std::ostream& outputStream) const
Expand Down
6 changes: 3 additions & 3 deletions src/simplnx/DataStructure/Geometry/IGeometry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -193,13 +193,13 @@ std::string IGeometry::LengthUnitToString(LengthUnit unit)
return "Unknown";
}

void IGeometry::checkUpdatedIdsImpl(const std::vector<std::pair<IdType, IdType>>& updatedIds)
void IGeometry::checkUpdatedIdsImpl(const std::unordered_map<DataObject::IdType, DataObject::IdType>& updatedIdsMap)
{
BaseGroup::checkUpdatedIdsImpl(updatedIds);
BaseGroup::checkUpdatedIdsImpl(updatedIdsMap);

std::vector<bool> visited(1, false);

for(const auto& updatedId : updatedIds)
for(const auto& updatedId : updatedIdsMap)
{
m_ElementSizesId = nx::core::VisitDataStructureId(m_ElementSizesId, updatedId, visited, 0);
}
Expand Down
4 changes: 2 additions & 2 deletions src/simplnx/DataStructure/Geometry/IGeometry.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -192,9 +192,9 @@ class SIMPLNX_EXPORT IGeometry : public BaseGroup

/**
* @brief Updates the array IDs. Should only be called by DataObject::checkUpdatedIds.
* @param updatedIds
* @param updatedIdsMap
*/
void checkUpdatedIdsImpl(const std::vector<std::pair<IdType, IdType>>& updatedIds) override;
void checkUpdatedIdsImpl(const std::unordered_map<DataObject::IdType, DataObject::IdType>& updatedIdsMap) override;

std::optional<IdType> m_ElementSizesId;

Expand Down
6 changes: 3 additions & 3 deletions src/simplnx/DataStructure/Geometry/IGridGeometry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,13 +74,13 @@ void IGridGeometry::setCellData(OptionalId id)
m_CellDataId = id;
}

void IGridGeometry::checkUpdatedIdsImpl(const std::vector<std::pair<IdType, IdType>>& updatedIds)
void IGridGeometry::checkUpdatedIdsImpl(const std::unordered_map<DataObject::IdType, DataObject::IdType>& updatedIdsMap)
{
IGeometry::checkUpdatedIdsImpl(updatedIds);
IGeometry::checkUpdatedIdsImpl(updatedIdsMap);

std::vector<bool> visited(1, false);

for(const auto& updatedId : updatedIds)
for(const auto& updatedId : updatedIdsMap)
{
m_CellDataId = nx::core::VisitDataStructureId(m_CellDataId, updatedId, visited, 0);
if(visited[0])
Expand Down
4 changes: 2 additions & 2 deletions src/simplnx/DataStructure/Geometry/IGridGeometry.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -225,9 +225,9 @@ class SIMPLNX_EXPORT IGridGeometry : public IGeometry

/**
* @brief Updates the array IDs. Should only be called by DataObject::checkUpdatedIds.
* @param updatedIds
* @param updatedIdsMap
*/
void checkUpdatedIdsImpl(const std::vector<std::pair<IdType, IdType>>& updatedIds) override;
void checkUpdatedIdsImpl(const std::unordered_map<DataObject::IdType, DataObject::IdType>& updatedIdsMap) override;

std::optional<IdType> m_CellDataId;
};
Expand Down
6 changes: 3 additions & 3 deletions src/simplnx/DataStructure/Geometry/INodeGeometry0D.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -240,13 +240,13 @@ void INodeGeometry0D::setVertexAttributeMatrix(const AttributeMatrix& attributeM
m_VertexAttributeMatrixId = attributeMatrix.getId();
}

void INodeGeometry0D::checkUpdatedIdsImpl(const std::vector<std::pair<IdType, IdType>>& updatedIds)
void INodeGeometry0D::checkUpdatedIdsImpl(const std::unordered_map<DataObject::IdType, DataObject::IdType>& updatedIdsMap)
{
IGeometry::checkUpdatedIdsImpl(updatedIds);
IGeometry::checkUpdatedIdsImpl(updatedIdsMap);

std::vector<bool> visited(2, false);

for(const auto& updatedId : updatedIds)
for(const auto& updatedId : updatedIdsMap)
{
m_VertexDataArrayId = nx::core::VisitDataStructureId(m_VertexDataArrayId, updatedId, visited, 0);
m_VertexAttributeMatrixId = nx::core::VisitDataStructureId(m_VertexAttributeMatrixId, updatedId, visited, 1);
Expand Down
4 changes: 2 additions & 2 deletions src/simplnx/DataStructure/Geometry/INodeGeometry0D.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -186,9 +186,9 @@ class SIMPLNX_EXPORT INodeGeometry0D : public IGeometry

/**
* @brief Updates the array IDs. Should only be called by DataObject::checkUpdatedIds.
* @param updatedIds
* @param updatedIdsMap
*/
void checkUpdatedIdsImpl(const std::vector<std::pair<IdType, IdType>>& updatedIds) override;
void checkUpdatedIdsImpl(const std::unordered_map<DataObject::IdType, DataObject::IdType>& updatedIdsMap) override;

/* ***************************************************************************
* These variables are the Ids of the arrays from the DataStructure object.
Expand Down
6 changes: 3 additions & 3 deletions src/simplnx/DataStructure/Geometry/INodeGeometry1D.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -258,13 +258,13 @@ void INodeGeometry1D::setElementSizesId(const std::optional<IdType>& sizesId)
m_ElementSizesId = sizesId;
}

void INodeGeometry1D::checkUpdatedIdsImpl(const std::vector<std::pair<IdType, IdType>>& updatedIds)
void INodeGeometry1D::checkUpdatedIdsImpl(const std::unordered_map<DataObject::IdType, DataObject::IdType>& updatedIdsMap)
{
INodeGeometry0D::checkUpdatedIdsImpl(updatedIds);
INodeGeometry0D::checkUpdatedIdsImpl(updatedIdsMap);

std::vector<bool> visited(7, false);

for(const auto& updatedId : updatedIds)
for(const auto& updatedId : updatedIdsMap)
{
m_EdgeAttributeMatrixId = nx::core::VisitDataStructureId(m_EdgeAttributeMatrixId, updatedId, visited, 0);
m_EdgeDataArrayId = nx::core::VisitDataStructureId(m_EdgeDataArrayId, updatedId, visited, 1);
Expand Down
4 changes: 2 additions & 2 deletions src/simplnx/DataStructure/Geometry/INodeGeometry1D.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -223,9 +223,9 @@ class SIMPLNX_EXPORT INodeGeometry1D : public INodeGeometry0D

/**
* @brief Updates the array IDs. Should only be called by DataObject::checkUpdatedIds.
* @param updatedIds
* @param updatedIdsMap
*/
void checkUpdatedIdsImpl(const std::vector<std::pair<IdType, IdType>>& updatedIds) override;
void checkUpdatedIdsImpl(const std::unordered_map<DataObject::IdType, DataObject::IdType>& updatedIdsMap) override;

/* ***************************************************************************
* These variables are the Ids of the arrays from the DataStructure object.
Expand Down
6 changes: 3 additions & 3 deletions src/simplnx/DataStructure/Geometry/INodeGeometry2D.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -212,11 +212,11 @@ INodeGeometry2D::SharedEdgeList* INodeGeometry2D::createSharedEdgeList(usize num
return edges;
}

void INodeGeometry2D::checkUpdatedIdsImpl(const std::vector<std::pair<IdType, IdType>>& updatedIds)
void INodeGeometry2D::checkUpdatedIdsImpl(const std::unordered_map<DataObject::IdType, DataObject::IdType>& updatedIdsMap)
{
INodeGeometry1D::checkUpdatedIdsImpl(updatedIds);
INodeGeometry1D::checkUpdatedIdsImpl(updatedIdsMap);
std::vector<bool> visited(3, false);
for(const auto& updatedId : updatedIds)
for(const auto& updatedId : updatedIdsMap)
{
m_FaceListId = nx::core::VisitDataStructureId(m_FaceListId, updatedId, visited, 0);
m_FaceAttributeMatrixId = nx::core::VisitDataStructureId(m_FaceAttributeMatrixId, updatedId, visited, 1);
Expand Down
4 changes: 2 additions & 2 deletions src/simplnx/DataStructure/Geometry/INodeGeometry2D.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -211,9 +211,9 @@ class SIMPLNX_EXPORT INodeGeometry2D : public INodeGeometry1D

/**
* @brief Updates the array IDs. Should only be called by DataObject::checkUpdatedIds.
* @param updatedIds
* @param updatedIdsMap
*/
void checkUpdatedIdsImpl(const std::vector<std::pair<IdType, IdType>>& updatedIds) override;
void checkUpdatedIdsImpl(const std::unordered_map<DataObject::IdType, DataObject::IdType>& updatedIdsMap) override;

/* ***************************************************************************
* These variables are the Ids of the arrays from the DataStructure object.
Expand Down
6 changes: 3 additions & 3 deletions src/simplnx/DataStructure/Geometry/INodeGeometry3D.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -222,12 +222,12 @@ INodeGeometry3D::SharedTriList* INodeGeometry3D::createSharedTriList(usize numTr
return triangles;
}

void INodeGeometry3D::checkUpdatedIdsImpl(const std::vector<std::pair<IdType, IdType>>& updatedIds)
void INodeGeometry3D::checkUpdatedIdsImpl(const std::unordered_map<DataObject::IdType, DataObject::IdType>& updatedIdsMap)
{
INodeGeometry2D::checkUpdatedIdsImpl(updatedIds);
INodeGeometry2D::checkUpdatedIdsImpl(updatedIdsMap);
std::vector<bool> visited(3, false);

for(const auto& updatedId : updatedIds)
for(const auto& updatedId : updatedIdsMap)
{
m_PolyhedronListId = nx::core::VisitDataStructureId(m_PolyhedronListId, updatedId, visited, 0);
m_PolyhedronAttributeMatrixId = nx::core::VisitDataStructureId(m_PolyhedronAttributeMatrixId, updatedId, visited, 1);
Expand Down
4 changes: 2 additions & 2 deletions src/simplnx/DataStructure/Geometry/INodeGeometry3D.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -205,9 +205,9 @@ class SIMPLNX_EXPORT INodeGeometry3D : public INodeGeometry2D

/**
* @brief Updates the array IDs. Should only be called by DataObject::checkUpdatedIds.
* @param updatedIds
* @param updatedIdsMap
*/
void checkUpdatedIdsImpl(const std::vector<std::pair<IdType, IdType>>& updatedIds) override;
void checkUpdatedIdsImpl(const std::unordered_map<DataObject::IdType, DataObject::IdType>& updatedIdsMap) override;

/* ***************************************************************************
* These variables are the Ids of the arrays from the DataStructure object.
Expand Down
6 changes: 3 additions & 3 deletions src/simplnx/DataStructure/Geometry/RectGridGeom.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -700,12 +700,12 @@ std::optional<usize> RectGridGeom::getIndex(float64 xCoord, float64 yCoord, floa
return (ySize * xSize * z) + (xSize * y) + x;
}

void RectGridGeom::checkUpdatedIdsImpl(const std::vector<std::pair<IdType, IdType>>& updatedIds)
void RectGridGeom::checkUpdatedIdsImpl(const std::unordered_map<DataObject::IdType, DataObject::IdType>& updatedIdsMap)
{
IGridGeometry::checkUpdatedIdsImpl(updatedIds);
IGridGeometry::checkUpdatedIdsImpl(updatedIdsMap);
std::vector<bool> visited(3, false);

for(const auto& updatedId : updatedIds)
for(const auto& updatedId : updatedIdsMap)
{
m_xBoundsId = nx::core::VisitDataStructureId(m_xBoundsId, updatedId, visited, 0);
m_yBoundsId = nx::core::VisitDataStructureId(m_yBoundsId, updatedId, visited, 1);
Expand Down
Loading

0 comments on commit 8efcc3a

Please sign in to comment.