Skip to content

Commit 8381d1d

Browse files
imikejacksonclaude
andauthored
ENH: Move Filter executeImpl() logic to Algorithm classes (#1544)
ENH: Move non-trivial Filter executeImpl logic to Algorithm classes (#1284) Convert 24 filters to use the Algorithm class pattern by moving execute logic from Filter files into dedicated Algorithm classes. Each filter now extracts parameters into an InputValues struct and delegates to an Algorithm operator()(). This improves separation of concerns and follows the established codebase pattern. Filters converted: ChangeAngleRepresentation, ComputeFeatureNeighbors, ComputeFeaturePhases, ComputeFeaturePhasesBinary, CopyFeatureArrayToElementArray, CreateFeatureArrayFromElementArray, CreateGeometry, CropImageGeometry, CropVertexGeometry, ExtractPipelineToFile, InitializeImageGeomCellData, InterpolatePointCloudToRegularGrid, MultiThresholdObjects, ReadHDF5Dataset, ReadTextDataArray, RemoveFlaggedVertices, ReverseTriangleWinding, RobustAutomaticThreshold, TriangleDihedralAngle, TriangleNormal, WriteASCIIData, WriteBinaryData, WriteDREAM3D, WriteFeatureDataCSV Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 30c9b10 commit 8381d1d

100 files changed

Lines changed: 3539 additions & 3228 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

CLAUDE.md

Lines changed: 308 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,308 @@
1+
# Project Guidelines for Claude
2+
3+
## Project Overview
4+
5+
[SIMPLNX Issue 1284](https://github.com/BlueQuartzSoftware/simplnx/issues/1284)
6+
I would like to work on this issue more by converting Filters that have anything more than a trivial execute implementation to move that implementation to an "Algorithm" class like the bulk of the other filters.
7+
8+
The Algorithm files are located in src/Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/not_used.
9+
10+
Look at the other filters and understand how we create the Algorithm classes and follow that same style.
11+
12+
Each Algorithm class that gets updated should be moved out of the "not_used" folder.
13+
14+
If anything is ambiguous please ask.
15+
16+
## Directory Structure
17+
- `src/simplnx/` - Core library (Common, Core, DataStructure, Filter, Parameters, Pipeline, Plugin, Utilities)
18+
- `src/Plugins/` - Plugin modules
19+
- `src/nxrunner/` - CLI runner
20+
- `test/` - Test files
21+
- `cmake/` - CMake configuration
22+
- Additional Plugin at "/Users/mjackson/Workspace1/DREAM3D_Plugins/SimplnxReview"
23+
- Additional Plugin at "/Users/mjackson/Workspace1/DREAM3D_Plugins/FileStore"
24+
- Additional Plugin at "/Users/mjackson/Workspace1/DREAM3D_Plugins/Synthetic"
25+
- DREAM3DNX application located in "/Users/mjackson/Workspace1/DREAM3DNX"
26+
27+
## Directories to Ignore
28+
- `scripts/` - Build/utility scripts
29+
- `conda/` - Conda packaging
30+
31+
## Coding Standards
32+
33+
### C++ Style (from .clang-format)
34+
- C++20 standard
35+
- Allman brace style (braces on new lines for classes, control statements, enums, functions, namespaces, structs, before else)
36+
- 200 column limit
37+
- 2-space indentation, no tabs
38+
- Pointer alignment left (`int* ptr` not `int *ptr`)
39+
- No space before parentheses
40+
- Sort includes alphabetically
41+
- No short functions on single line
42+
- Always break template declarations
43+
- Constructor initializers break before comma
44+
45+
### Naming Conventions (from .clang-tidy)
46+
- C++ header files: `.hpp` extension
47+
- C++ source files: `.cpp` extension
48+
- Namespaces: `lower_case`
49+
- Classes: `CamelCase`
50+
- Structs: `CamelCase`
51+
- Class methods: `camelBack`
52+
- Functions: `camelBack`
53+
- Variables: `camelBack`
54+
- Private members: `m_` prefix + `CamelCase` (e.g., `m_MemberVariable`)
55+
- Global variables: `CamelCase`
56+
- Global constants: `k_` prefix + `CamelCase` (e.g., `k_DefaultValue`)
57+
- Local pointers: `camelBack` + `Ptr` suffix (e.g., `dataPtr`)
58+
- Type aliases: `CamelCase` + `Type` suffix (e.g., `ValueType`)
59+
- Macros: `UPPER_CASE`
60+
61+
### Descriptive Variable Naming
62+
Use suffixes to make variable types and purposes immediately clear:
63+
64+
**Geometry variables use `Geom` suffix:**
65+
- Correct: `const auto& imageGeom = dataStructure.getDataRefAs<ImageGeom>(path);`
66+
- Incorrect: `const auto& image = dataStructure.getDataRefAs<ImageGeom>(path);`
67+
68+
**DataStore references use `Ref` suffix:**
69+
- Correct: `const auto& verticesRef = vertexGeom.getVertices()->getDataStoreRef();`
70+
- Incorrect: `const auto& vertices = vertexGeom.getVertices()->getDataStoreRef();`
71+
72+
Examples:
73+
```cpp
74+
// Geometry variables
75+
auto& imageGeom = dataStructure.getDataRefAs<ImageGeom>(imagePath);
76+
const auto& rectGridGeom = dataStructure.getDataRefAs<RectGridGeom>(rectPath);
77+
const auto& edgeGeom = dataStructure.getDataRefAs<EdgeGeom>(edgePath);
78+
79+
// DataStore references
80+
const auto& xBoundsRef = rectGridGeom.getXBounds()->getDataStoreRef();
81+
const auto& yBoundsRef = rectGridGeom.getYBounds()->getDataStoreRef();
82+
const auto& verticesRef = edgeGeom.getVertices()->getDataStoreRef();
83+
```
84+
85+
These conventions improve code clarity and distinguish between geometry objects and their underlying data references.
86+
87+
### File Organization
88+
- When creating a C++ based simplnx filter inside a plugin, the complete filter will have a "NameFilter.hpp" and "NameFilter.cpp" file, an "Algorithm/Name.hpp" and "Algorithm/Name.cpp".
89+
- Filter documentation files are created in Markdown and are in the "docs" subfolder inside the Plugins directory
90+
- Unit tests should be created in the 'test' subfolder and use the 'catch2' unit testing framework.
91+
92+
## Filter Implementation Guidelines
93+
94+
### Parameter Validation
95+
- Selection parameters (GeometrySelectionParameter, ArraySelectionParameter, DataGroupSelectionParameter, etc.) automatically validate that the selected object exists in the DataStructure. Do NOT add null checks for these in preflightImpl() or executeImpl().
96+
- Only add explicit existence checks for objects that are not validated by a selection parameter.
97+
98+
### DataStructure Access
99+
- Use `getDataRefAs<T>()` to get a reference when you know the object exists (e.g., validated by a selection parameter).
100+
- Use `getDataAs<T>()` to get a pointer only when you need to check if an object exists or when the object may not be present.
101+
- **IMPORTANT**: In unit tests, always wrap `getDataRefAs<T>()` calls with `REQUIRE_NOTHROW()` to provide clear test failure messages if the object doesn't exist.
102+
103+
Example - Correct:
104+
```cpp
105+
// Parameter already validated this exists, use reference
106+
const auto& imageGeom = dataStructure.getDataRefAs<ImageGeom>(pInputImageGeometryPathValue);
107+
SizeVec3 dims = imageGeom.getDimensions();
108+
```
109+
110+
Example - Incorrect:
111+
```cpp
112+
// Unnecessary null check - parameter already validated existence
113+
const auto* imageGeomPtr = dataStructure.getDataAs<ImageGeom>(pInputImageGeometryPathValue);
114+
if(imageGeomPtr == nullptr)
115+
{
116+
return {MakeErrorResult<OutputActions>(-1000, "Could not find geometry")};
117+
}
118+
```
119+
120+
## Thread Safety
121+
122+
### DataArray and DataStore Classes Are NOT Thread-Safe
123+
- `DataArray`, `DataStore`, and `AbstractDataStore` classes are **NOT thread-safe** for concurrent read or write access.
124+
- The subscript operator (`operator[]`) and other access methods may have internal state or go through virtual function calls that are not safe for concurrent access, even when accessing different indices.
125+
- Some `DataStore` subclasses use out-of-core implementations where data may not be resident in memory. Getting raw pointers to the underlying data is dangerous and should be avoided.
126+
127+
### Parallelization Guidelines
128+
- When writing parallel algorithms using `ParallelDataAlgorithm`, be aware that passing `DataArray` or `DataStore` references to worker classes can cause random failures on different platforms.
129+
- If parallel access to data arrays is required, consider:
130+
1. Disabling parallelization with `parallelAlgorithm.setParallelizationEnabled(false)` for correctness
131+
2. Using thread-local storage for intermediate results
132+
3. Structuring the algorithm to avoid concurrent access to the same DataArray
133+
- Do NOT assume that writing to different indices of a DataArray from multiple threads is safe.
134+
135+
Example - Potentially Unsafe:
136+
```cpp
137+
// This pattern can cause random failures even when threads write to different indices
138+
class MyParallelWorker
139+
{
140+
DataArray<float32>& m_OutputArray; // NOT thread-safe for concurrent access
141+
void operator()(const Range& range) const
142+
{
143+
for(usize i = range.min(); i < range.max(); i++)
144+
{
145+
m_OutputArray[i] = computeValue(i); // May fail randomly
146+
}
147+
}
148+
};
149+
```
150+
151+
## Build System
152+
- vcpkg for dependency management
153+
- CMake-based build system
154+
155+
Example configuring the project
156+
```bash
157+
cd /Users/mjackson/Workspace2/simplnx && cmake --preset simplnx-Rel
158+
```
159+
160+
- Build directory is located at "/Users/mjackson/Workspace2/DREAM3D-Build/simplnx-Rel"
161+
162+
Example building the project
163+
```bash
164+
cd /Users/mjackson/Workspace2/DREAM3D-Build/simplnx-Rel && cmake --build . --target all
165+
```
166+
167+
Ensuring all test data files are downloaded
168+
```bash
169+
cd /Users/mjackson/Workspace2/DREAM3D-Build/simplnx-Rel && cmake --build . --target Fetch_Remote_Data_Files
170+
```
171+
172+
- Python anaconda environment 'dream3d' can be used if needed
173+
174+
## Testing
175+
- Unit tests use the Catch2 framework.
176+
- Each `TEST_CASE` should include `UnitTest::CheckArraysInheritTupleDims(dataStructure);` near the end of the test to ensure all created data arrays have correct tuple dimensions inherited from their parent groups.
177+
- Use the `ctest` to run unit tests
178+
179+
### Running Unit Tests
180+
- Always use `ctest` to run unit tests, NOT the test binary directly
181+
- The `ctest` command handles test data extraction and cleanup automatically
182+
- Use the `-R` flag to run specific tests by name pattern
183+
184+
Example - Running a specific test:
185+
```bash
186+
cd /Users/mjackson/Workspace2/DREAM3D-Build/NX-Com-Qt69-Vtk95-Rel && ctest -R "SimplnxCore::FillBadData" --verbose
187+
```
188+
189+
Example - Running all SimplnxCore tests:
190+
```bash
191+
cd /Users/mjackson/Workspace2/DREAM3D-Build/NX-Com-Qt69-Vtk95-Rel && ctest -R "SimplnxCore::" --verbose
192+
```
193+
194+
### Printing debug statements in unit tests
195+
196+
Example - Correct
197+
198+
auto executeResult = filter.execute(dataStructure, args, nullptr, IFilter::MessageHandler{[](const IFilter::Message& message){ fmt::print("{}\n", message.message); }});
199+
200+
### Exemplar-Based Testing Pattern
201+
- Many tests use "exemplar" datasets - pre-generated golden reference data stored in `.dream3d` files
202+
- Exemplar datasets are generated by running pipeline files (`.d3dpipeline`) that configure and execute filters
203+
204+
#### Workflow for Creating and Publishing Test Data:
205+
1. **Generate test data locally**: Create pipeline file with filter configurations and `WriteDREAM3DFilter` to save results
206+
2. **Execute pipeline**: Run the pipeline to generate exemplar `.dream3d` file and any input data files
207+
3. **Package as tar.gz**: Compress test data (no `6_6_` prefix needed - that was only for legacy DREAM3D data)
208+
```bash
209+
tar -zvcf test_name.tar.gz test_directory/
210+
```
211+
4. **Compute SHA512 hash**:
212+
```bash
213+
shasum -a 512 test_name.tar.gz
214+
```
215+
5. **Upload to GitHub**: Upload to the [DREAM3D data archive release](https://github.com/BlueQuartzSoftware/simplnx/releases/tag/Data_Archive)
216+
6. **Update CMakeLists.txt**: Add `download_test_data()` call in `src/Plugins/[PluginName]/test/CMakeLists.txt`:
217+
```cmake
218+
download_test_data(DREAM3D_DATA_DIR ${DREAM3D_DATA_DIR}
219+
ARCHIVE_NAME test_name.tar.gz
220+
SHA512 <hash_from_step_4>)
221+
```
222+
7. **Test data auto-downloads**: When tests run, the sentinel mechanism automatically downloads and extracts the tar.gz to `unit_test::k_TestFilesDir`
223+
224+
#### Test Data Archive Naming and Versioning:
225+
- **Base naming**: Use descriptive names that match the test: `test_name.tar.gz`
226+
- **Version suffixes**: When updating existing test data, append version numbers: `test_name_v2.tar.gz`, `test_name_v3.tar.gz`
227+
- **When to version**:
228+
- Original archive already exists in GitHub data archive
229+
- Test requirements changed (new exemplars, different parameters, additional data files)
230+
- Cannot overwrite original because other code may depend on it
231+
- CMakeLists.txt may reference both old and new versions for different tests
232+
- **Check before creating**: Browse the [Data_Archive release](https://github.com/BlueQuartzSoftware/simplnx/releases/tag/Data_Archive) to see if your test data name already exists
233+
- **Legacy prefixes**: The `6_6_` and `6_5_` prefixes are for data from legacy DREAM3D/SIMPL versions - do NOT use for new DREAM3DNX test data
234+
235+
#### Test Code Pattern:
236+
```cpp
237+
namespace
238+
{
239+
const std::string k_TestDataDirName = "test_name";
240+
const fs::path k_TestDataDir = fs::path(unit_test::k_TestFilesDir.view()) / k_TestDataDirName;
241+
const fs::path k_ExemplarFile = k_TestDataDir / "test_name.dream3d";
242+
const fs::path k_InputImageFile = k_TestDataDir / "input_file.tif";
243+
}
244+
```
245+
246+
**Example**: If `import_image_stack_test.tar.gz` exists in the archive and you need to upload updated test data with new exemplars, create `import_image_stack_test_v2.tar.gz`. Update CMakeLists.txt to reference the new version, and optionally keep the old version if other tests depend on it.
247+
248+
#### Comparing Test Results Against Exemplars:
249+
- **Load exemplar DataStructure**: Use `UnitTest::LoadDataStructure(exemplarFilePath)` to load the .dream3d file
250+
- **ALWAYS use `REQUIRE_NOTHROW()` before `getDataRefAs<T>()`**: This applies to ALL `getDataRefAs` calls - both generated and exemplar data
251+
- **Get generated data**: Use `getDataRefAs<T>()` wrapped in `REQUIRE_NOTHROW()` since objects were just created by the filter
252+
- **Get exemplar data**: Use `getDataRefAs<T>()` wrapped in `REQUIRE_NOTHROW()` to verify the exemplar exists before accessing
253+
- **Compare geometries**: Use `UnitTest::CompareImageGeometry(&exemplarGeom, &generatedGeom)` - takes two pointers
254+
- **Compare arrays**: Use `UnitTest::CompareDataArrays<T>(exemplarArray, generatedArray)` - type-specific template
255+
- **Switch on data type** when comparing arrays to handle different types (uint8, uint16, uint32, float32, etc.)
256+
257+
Example pattern:
258+
```cpp
259+
// Load exemplar
260+
DataStructure exemplarDS = UnitTest::LoadDataStructure(k_ExemplarFile);
261+
262+
// Get geometries - ALWAYS wrap getDataRefAs with REQUIRE_NOTHROW
263+
REQUIRE_NOTHROW(dataStructure.getDataRefAs<ImageGeom>(generatedGeomPath));
264+
const auto& generatedGeom = dataStructure.getDataRefAs<ImageGeom>(generatedGeomPath);
265+
REQUIRE_NOTHROW(exemplarDS.getDataRefAs<ImageGeom>(DataPath({exemplarGeomName})));
266+
const auto& exemplarGeom = exemplarDS.getDataRefAs<ImageGeom>(DataPath({exemplarGeomName}));
267+
268+
// Compare geometries (dimensions, origin, spacing) - pass pointers
269+
UnitTest::CompareImageGeometry(&exemplarGeom, &generatedGeom);
270+
271+
// Get arrays - ALWAYS wrap getDataRefAs with REQUIRE_NOTHROW
272+
REQUIRE_NOTHROW(dataStructure.getDataRefAs<IDataArray>(generatedDataPath));
273+
const auto& generatedArray = dataStructure.getDataRefAs<IDataArray>(generatedDataPath);
274+
REQUIRE_NOTHROW(exemplarDS.getDataRefAs<IDataArray>(exemplarDataPath));
275+
const auto& exemplarArray = exemplarDS.getDataRefAs<IDataArray>(exemplarDataPath);
276+
277+
// Compare arrays based on type
278+
switch(generatedArray.getDataType())
279+
{
280+
case DataType::uint8:
281+
UnitTest::CompareDataArrays<uint8>(exemplarArray, generatedArray);
282+
break;
283+
case DataType::uint16:
284+
UnitTest::CompareDataArrays<uint16>(exemplarArray, generatedArray);
285+
break;
286+
// ... etc
287+
}
288+
```
289+
290+
**Important**: Use the standardized `UnitTest::` comparison methods directly in test code.
291+
292+
### Test Organization
293+
- Each test should call `UnitTest::LoadPlugins()` before executing filters
294+
- Use `DYNAMIC_SECTION()` for parameterized tests that generate multiple test cases
295+
296+
## Pipeline Files
297+
- JSON format with `.d3dpipeline` extension
298+
- Contains array of filter configurations with arguments
299+
- Each filter has:
300+
- `args`: Dictionary of parameter keys and values
301+
- `comments`: Description of what the filter does
302+
- `filter`: Name and UUID
303+
- `isDisabled`: Boolean to skip filter execution
304+
- Common pattern: Multiple filter configurations followed by WriteDREAM3DFilter to save all results to one `.dream3d` file
305+
- Output geometry paths in pipeline must match exemplar names expected by tests
306+
307+
## Additional Notes
308+
<!-- Add any other project-specific rules here -->

0 commit comments

Comments
 (0)