Skip to content
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.

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

BUG: Issues with "(08) Small IN100 Full Reconstruction" prebuilt pipeline and neighbor filters used #922

Closed
2 tasks done
StopkaKris opened this issue Apr 19, 2024 · 8 comments
Labels
bug Something isn't working In Progress A developer is currently working on this

Comments

@StopkaKris
Copy link

Is there an existing issue for this?

  • I have searched the existing issues, known issues in release notes, and documentation.

Brief Description of the Issue and Expected Behavior

I came across these issues while using the prebuilt "(08) Small IN100 Full Reconstruction" pipeline. Please see below to reproduce and let me know if there are any questions. This was found using DREAM3DNX Version 7.0.0-RC-10.

Best regards,
Krzysztof Stopka

Version

DREAM3D NX (version 7)

What operating system are you seeing the problem on? [Further details may be required during triage process]

Windows 10

What hardware architecture are you using? [Further details may be required during triage process]

x86_64

What section did you encounter the error in? [Further details may be required during triage process]

No response

Steps To Reproduce

  1. Run "(01) Small IN100 Archive" pipeline to create .h5ebsd file, then open "(08) Small IN100 Full Reconstruction".
  2. Add Write Feature Data as CSV File at the end of the pipeline. By default, the "Write Neighbor Data" box is checked, and the Data Structure contains "NeighborList" and "SharedSurfaceAreaList", so these should be written to the .csv file. However, no feature neighbor data is written to the .csv file when the pipeline is executed.
  3. To examine whether there is a problem with filter 16, Find Feature Neighbors, I added another instance of this filter directly after fitler 16. I reran the pipeline but Write Feature Data as CSV File still did not write any neighbor information to the .csv file. I tried toggling the "Write Neighbor Data" checkbox but this did not change anything.
  4. I added Find Feature Neighbor Misorientation and used the Feature Neighbor List generated by the first instance of Find Feature Neighbors. There was still no neighbor information written to the .csv file.
  5. I opened a new window of DREAM.3D NX and used Read DREAM.3D File to load the .dream3d file saved by the previous pipeline, in an attempt to calculate feature misorientations after reading the data "fresh" from the .dream3d file. However, the NeighborList was no longer in the CellFeatureData Attribute Matrix.
  6. I added Find Feature Neighbors, Find Feature Neighbor Misorientations, and Write Feature Data as CSV File as filters 2, 3, and 4, respectively. After running this pipeline, the .csv file contained the expected neighbor data.
  7. Upon inspection of the .csv file, I found that there were some feature IDs in the .csv file that contained 0 neighbors (e.g., grains 242, 402, 421, 549, etc.). This is because the "(08) Small IN100 Full Reconstruction" uses Remove Minimum Size Features but does not subsequently renumber the remaining features, so feature IDs are not consecutive.

To summarize the bugs I came across:

  1. When the Write Feature Data as CSV File was added to the end of the "(08) Small IN100 Full Reconstruction", it did not write feature neighbor information. I suspect it is something about filter 16, Find Feature Neighbors, in this pipeline.
  2. Write DREAM3D NX File did not write the "Neighbor List Info" arrays at the end of "(08) Small IN100 Full Reconstruction". Although the "Neighbor List Info" arrays appear in the Data Structure, they may not actually exist and are therefore not written.
  3. This is technically not a bug, but it may confuse users to see feature IDs with zero neighbors due to grains being removed through the use of filters such as Remove Minimum Size Features and Minimum Number of Neighbors, particularly because this is the default organization of the "(08) Small IN100 Full Reconstruction" prebuilt pipeline. Would it make sense to edit this pipeline so that feature IDs and their properties are reassigned/recomputed after any features are removed due to minimum size or number of neighbors?

Relevant log output

No response

Anything else?

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct
@StopkaKris StopkaKris added the bug Something isn't working label Apr 19, 2024
@nyoungbq
Copy link
Contributor

Hello Krzysztof,

I was able to replicate this bug on my Linux machine, so I will look into it as soon as I get in Tuesday.

Nathan

@nyoungbq nyoungbq added the In Progress A developer is currently working on this label Apr 19, 2024
@imikejackson
Copy link
Contributor

@StopkaKris Looking into this a bit more, looks like filter number 17 "Minimum Number of Neighbors" is removing the neighbor lists without a warning because the featureIds are being modified in that filter. You would need to recalculate the neighbor list again after that filter.
So after "Minimum Number of Neighbors" insert "Find Feature Neighbors" (again) into the pipeline. This isn't very user friendly at the moment. In trying to fix it, I also came across an API issue that I need to discuss with the team before getting a "better" fix put in.

I remember a long time ago discussing these filters with Mike Groeber: Should we just calculate the neighbor lists on-demand but that also caused a boat load of issues. So we ended up with the situation that we are in now.

Thanks for all the bug reports. Keep them coming. Eventually they will all dry up. :-)

@imikejackson
Copy link
Contributor

#926

@imikejackson
Copy link
Contributor

[2024:04:23 18:51:20] [14/22] Find Feature Sizes: End
[2024:04:23 18:51:20] [15/22] Remove Minimum Size Features: Begin
[2024:04:23 18:51:20] [15/22] Remove Minimum Size Features: Feature Count Changed: Previous: 5416 New: 2355
[2024:04:23 18:51:20] [15/22] Remove Minimum Size Features: NeighborList 'DataContainer/CellFeatureData/NeighborList2' was removed from the DataStructure.
[2024:04:23 18:51:20] [15/22] Remove Minimum Size Features: NeighborList 'DataContainer/CellFeatureData/SharedSurfaceAreaList2' was removed from the DataStructure.
[2024:04:23 18:51:20] [15/22] Remove Minimum Size Features: End
[2024:04:23 18:51:20] [16/22] Find Feature Neighbors: Begin
[2024:04:23 18:51:21] [16/22] Find Feature Neighbors: End
[2024:04:23 18:51:21] [17/22] Minimum Number of Neighbors: Begin
[2024:04:23 18:51:21] [17/22] Minimum Number of Neighbors: Feature Count Changed: Previous: 2355 New: 2353
[2024:04:23 18:51:21] [17/22] Minimum Number of Neighbors: NeighborList 'DataContainer/CellFeatureData/NeighborList' was removed from the DataStructure.
[2024:04:23 18:51:21] [17/22] Minimum Number of Neighbors: NeighborList 'DataContainer/CellFeatureData/SharedSurfaceAreaList' was removed from the DataStructure.
[2024:04:23 18:51:21] [17/22] Minimum Number of Neighbors: End

This what the output is looking like now. There are additional warnings during preflight for those filters.
Screenshot 2024-04-23 at 18 52 39

@StopkaKris
Copy link
Author

Looks great, thanks guys!

@imikejackson
Copy link
Contributor

imikejackson commented Apr 24, 2024

After internal discussions, I think the safest approach to this problem is to delete NeighborLists during the course of the filter with a change of wording of the filter to state that we are GOING to remove those NeighborLists. Since, currently, we can't know what filter created those NeighborLists in the first place, we can't include that information into the warning message.

Any other way we do this, the user will probably get themselves into a situation where they either (1) try to use a deleted NeighborList or (2) if we don't delete them, use an invalid NeighborList object. Based on this, just flat out deleting the NeighborLists is probably the safest thing to do.

There are a number of items that need to be updated to accomplish this goal:

  • Update DataGroupUtilities::RemoveInactiveObjects to defer deletion of the NeighborLists to the actual actions
  • Update MinNeighbors, RemoveMinimumSizeFeatures, RemoveFlaggedFeatures to emit the correct warning.
  • Update those filters to use a common Utility method to generate that warning
  • Update SamplingUtils::RenumberFeatures as needed since it uses DataGroupUtilities::RemoveInactiveObjects
  • Update any prebuilt pipelines
  • Update CropImageGeometry to emit a warning about NeighborLIsts (since it uses SamplingUtils::RenumberFeatures)
    Already emits a warning to state that NeighborLists will not be copied over.
  • Update ResampleImageGeom to emit a warning about NeighborLIsts (since it uses SamplingUtils::RenumberFeatures)
    Already emits a warning to state that NeighborLists will not be copied over.
  • Update documentation for all filters effected by this change to make it clear that we are deleting ANY NeighborList and why we are deleting them in the first place.
  • Update DeleteDataAction::TerminateNode() to return an error instead of a warning. (Reverse earlier change)

@imikejackson
Copy link
Contributor

Filters that Create A NeighborList (or copy from another DataStucture Group)

DREAM3D_Plugins/SimplnxReview/src/SimplnxReview/Filters/FindGroupingDensityFilter.cpp
simplnx/src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/FindFeatureNeighborCAxisMisalignmentsFilter.cpp
simplnx/src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/FindMisorientationsFilter.cpp
simplnx/src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/FindSlipTransmissionMetricsFilter.cpp
simplnx/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/AppendImageGeometryZSliceFilter.cpp
simplnx/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/FindFeatureClusteringFilter.cpp
simplnx/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/FindNeighborhoodsFilter.cpp
simplnx/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/FindNeighbors.cpp
simplnx/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/InterpolatePointCloudToRegularGridFilter.cpp
simplnx/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/NearestPointFuseRegularGridsFilter.cpp
simplnx/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/ResampleRectGridToImageGeomFilter.cpp
simplnx/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/FindArrayStatisticsFilter.cpp

These filters USE a Neighbor List

DREAM3D_Plugins/SimplnxReview/src/SimplnxReview/Filters/FindGroupingDensityFilter.cpp
DREAM3D_Plugins/SimplnxReview/src/SimplnxReview/Filters/FindLocalAverageCAxisMisalignmentsFilter.cpp
DREAM3D_Plugins/SimplnxReview/src/SimplnxReview/Filters/GroupMicroTextureRegionsFilter.cpp
DREAM3D_Plugins/SimplnxReview/src/SimplnxReview/Filters/MergeColoniesFilter.cpp
simplnx/src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/FindFeatureNeighborCAxisMisalignmentsFilter.cpp
simplnx/src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/FindMisorientationsFilter.cpp
simplnx/src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/FindSlipTransmissionMetricsFilter.cpp
simplnx/src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/MergeTwinsFilter.cpp
simplnx/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/FindNeighborListStatistics.cpp

@imikejackson
Copy link
Contributor

Fixed by #926

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working In Progress A developer is currently working on this
Projects
None yet
Development

No branches or pull requests

3 participants