Repository navigation
BUG: Numeric filters can throw after accepting nonnumeric arrays #1821
Description
Problem
Numeric-only filters can accept a StringArray or NeighborList through ArraySelectionParameter and then throw during preflight. The selection should produce an invalid preflight result with an actionable diagnostic before the filter accesses a numeric array.
Verified at simplnx commit 58f9f72c8917a2e07361048549e355fb81960221 on Windows with MSVC 2022, Release, Catch2 2.13.6. In ConvertDataFilter, both array kinds produced an unexpected Bad dynamic_cast! exception from filter.preflight(...). This report establishes an exception escaping the filter API; application-level handling was not tested.
Steps to Reproduce
- Check out the reported commit with testing enabled.
- Add the complete test source below to
src/Plugins/SimplnxCore/test/ConvertDataTest.cpp, using the existing test runner. Keep the existing tests in that file. - Build
SimplnxCoreUnitTestin Release. - Run the conversion rejection test through CTest:
cmake --build <build> --config Release --target SimplnxCoreUnitTest
ctest --test-dir <build> -C Release -R "ArraySelectionIssue: conversion rejects unsupported kinds without throwing" -j 1 --output-on-failure
The fixture creates two-tuples of numeric arrays, strings, and neighbor lists. No external input files are required. Each rejection section selects one nonnumeric array for Convert AttributeArray DataType and calls preflight.
Cause
ArraySelectionParameter::validatePath accepts any IArray, then applies allowed numeric types, component shapes, and storage-location checks only inside the IDataArray branch. StringArray and NeighborList skip that branch and return success.
ConvertDataFilter declares this generic selection parameter with all numeric types. Its preflight implementation then calls getDataRefAs<IDataArray>. That accessor uses a reference dynamic_cast, which throws for either nonnumeric kind. IFilter::preflight calls preflightImpl without catching this exception.
Generic consumers need different acceptance rules. ReshapeDataArrayFilter and SplitDataArrayByTupleFilter deliberately support StringArray and NeighborList. A blanket IDataArray requirement in the shared parameter would remove supported behavior.
Verification Unit Test
The complete source below contains three passing compatibility controls, one failing conversion test with two sections, and two additional policy probes. The policy probes document currently bypassed restrictions; the final cross-kind type/shape contract remains a design decision.
Observed results from an isolated published-head checkout and a snapshot of its previously validated Release libraries:
- Numeric type/shape validation, numeric conversion preflight, and generic StringArray/NeighborList reshape controls passed.
- The conversion test failed in both sections: 15 assertions, 13 passed, 2 failed.
- The shape/element-type probes failed three assertions: fixed-one-component StringArray and NeighborList selections passed a three-component restriction, and a float NeighborList passed an int32-only restriction. Their controls passed.
Actual conversion failure excerpt (the same failure occurred for each array kind):
REQUIRE_NOTHROW( result = filter.preflight(data, args) )
due to unexpected exception with message:
Bad dynamic_cast!
test cases: 1 | 0 passed | 1 failed
assertions: 15 | 13 passed | 2 failed
The isolated harness was built from the reported worktree headers and a copied runtime/import-library set taken before unrelated review fixes were rebuilt. The tests ran sequentially through CTest. No production fix was applied.
Run all cases after adding this source to the normal test file:
ctest --test-dir <build> -C Release -R "ArraySelectionIssue:" -j 1 --output-on-failure
// Standalone Catch2 v2 regression source for simplnx SHA 58f9f72c8917a2e07361048549e355fb81960221.
// Compile with the existing SimplnxCoreUnitTest runner, or define CATCH_CONFIG_MAIN
// on this translation unit when linking a separate executable against SimplnxCore and simplnx.
#include "SimplnxCore/Filters/ConvertDataFilter.hpp"
#include "SimplnxCore/Filters/ReshapeDataArrayFilter.hpp"
#include "simplnx/Common/DataTypeUtilities.hpp"
#include "simplnx/DataStructure/DataArray.hpp"
#include "simplnx/DataStructure/DataStore.hpp"
#include "simplnx/DataStructure/DataStructure.hpp"
#include "simplnx/DataStructure/NeighborList.hpp"
#include "simplnx/DataStructure/StringArray.hpp"
#include "simplnx/Parameters/ArraySelectionParameter.hpp"
#include "simplnx/Parameters/DynamicTableParameter.hpp"
#include "simplnx/UnitTest/UnitTestCommon.hpp"
#include <catch2/catch.hpp>
using namespace nx::core;
namespace
{
void PopulateArrays(DataStructure& data)
{
REQUIRE(DataArray<int32>::CreateWithStore<DataStore<int32>>(data, "IntScalar", ShapeType{2}, ShapeType{1}) != nullptr);
REQUIRE(DataArray<int32>::CreateWithStore<DataStore<int32>>(data, "IntVector", ShapeType{2}, ShapeType{3}) != nullptr);
REQUIRE(DataArray<float32>::CreateWithStore<DataStore<float32>>(data, "FloatScalar", ShapeType{2}, ShapeType{1}) != nullptr);
REQUIRE(StringArray::CreateWithValues(data, "Strings", ShapeType{2}, {"alpha", "beta"}) != nullptr);
auto* ints = NeighborList<int32>::Create(data, "IntNeighbors", ShapeType{2});
REQUIRE(ints != nullptr);
ints->setList(0, std::vector<int32>{1, 7});
ints->setList(1, std::vector<int32>{3});
REQUIRE(NeighborList<float32>::Create(data, "FloatNeighbors", ShapeType{2}) != nullptr);
}
} // namespace
TEST_CASE("ArraySelectionIssue: numeric restrictions remain enforced", "[ArraySelectionIssue][controls]")
{
DataStructure data;
PopulateArrays(data);
const ArraySelectionParameter scalar("input", "Input", "", {}, {DataType::int32}, {{1}});
CHECK(scalar.validatePath(data, DataPath({"IntScalar"})).valid());
CHECK(scalar.validatePath(data, DataPath({"FloatScalar"})).invalid());
CHECK(scalar.validatePath(data, DataPath({"IntVector"})).invalid());
}
// Exploratory policy probes: these assert one possible common shape/element-type
// contract. They do not encode an approved API change or blanket kind rejection.
TEST_CASE("ArraySelectionIssue: shared component shape restriction", "[ArraySelectionIssue][regression][policy]")
{
DataStructure data;
PopulateArrays(data);
const ArraySelectionParameter vector("input", "Input", "", {}, GetAllDataTypes(), {{3}});
CHECK(vector.validatePath(data, DataPath({"IntVector"})).valid());
CHECK(vector.validatePath(data, DataPath({"IntScalar"})).invalid());
SECTION("StringArray has one component")
{
REQUIRE_NOTHROW(data.getDataRefAs<StringArray>(DataPath({"Strings"})));
REQUIRE(data.getDataRefAs<StringArray>(DataPath({"Strings"})).getComponentShape() == ShapeType{1});
CHECK(vector.validatePath(data, DataPath({"Strings"})).invalid());
}
SECTION("NeighborList has one component")
{
REQUIRE_NOTHROW(data.getDataRefAs<NeighborList<int32>>(DataPath({"IntNeighbors"})));
REQUIRE(data.getDataRefAs<NeighborList<int32>>(DataPath({"IntNeighbors"})).getComponentShape() == ShapeType{1});
CHECK(vector.validatePath(data, DataPath({"IntNeighbors"})).invalid());
}
}
TEST_CASE("ArraySelectionIssue: neighbor element type restriction", "[ArraySelectionIssue][regression][policy]")
{
DataStructure data;
PopulateArrays(data);
const ArraySelectionParameter ints("input", "Input", "", {}, {DataType::int32}, {{1}});
// Acceptance of matching NeighborList inputs is a compatibility choice. This
// test chooses preservation, then requires the declared element restriction.
CHECK(ints.validatePath(data, DataPath({"IntNeighbors"})).valid());
CHECK(ints.validatePath(data, DataPath({"FloatNeighbors"})).invalid());
}
TEST_CASE("ArraySelectionIssue: generic reshape keeps supported array kinds", "[ArraySelectionIssue][controls]")
{
UnitTest::LoadPlugins();
DataStructure data;
PopulateArrays(data);
const ArraySelectionParameter generic("input", "Input", "", {}, GetAllDataTypes());
CHECK(generic.validatePath(data, DataPath({"Strings"})).valid());
CHECK(generic.validatePath(data, DataPath({"IntNeighbors"})).valid());
ReshapeDataArrayFilter filter;
auto args = filter.getDefaultArguments();
args.insertOrAssign(ReshapeDataArrayFilter::k_TupleDims_Key, std::make_any<DynamicTableParameter::ValueType>(DynamicTableParameter::ValueType{{1.0, 2.0}}));
SECTION("StringArray remains supported")
{
const DataPath path({"Strings"});
args.insertOrAssign(ReshapeDataArrayFilter::k_Input_Array_Key, std::make_any<DataPath>(path));
REQUIRE(filter.preflight(data, args).outputActions.valid());
REQUIRE(filter.execute(data, args).result.valid());
REQUIRE_NOTHROW(data.getDataRefAs<StringArray>(path));
const auto& output = data.getDataRefAs<StringArray>(path);
CHECK(output.getTupleShape() == ShapeType{1, 2});
CHECK(output[0] == "alpha");
CHECK(output[1] == "beta");
}
SECTION("NeighborList remains supported")
{
const DataPath path({"IntNeighbors"});
args.insertOrAssign(ReshapeDataArrayFilter::k_Input_Array_Key, std::make_any<DataPath>(path));
REQUIRE(filter.preflight(data, args).outputActions.valid());
REQUIRE(filter.execute(data, args).result.valid());
REQUIRE_NOTHROW(data.getDataRefAs<NeighborList<int32>>(path));
const auto& output = data.getDataRefAs<NeighborList<int32>>(path);
CHECK(output.getTupleShape() == ShapeType{1, 2});
CHECK(output.getList(0) == std::vector<int32>{1, 7});
CHECK(output.getList(1) == std::vector<int32>{3});
}
UnitTest::CheckArraysInheritTupleDims(data);
}
TEST_CASE("ArraySelectionIssue: conversion accepts numeric arrays", "[ArraySelectionIssue][controls]")
{
UnitTest::LoadPlugins();
DataStructure data;
PopulateArrays(data);
ConvertDataFilter filter;
auto args = filter.getDefaultArguments();
args.insertOrAssign(ConvertDataFilter::k_ArrayToConvertPath_Key, std::make_any<DataPath>(DataPath({"IntScalar"})));
REQUIRE(filter.preflight(data, args).outputActions.valid());
}
TEST_CASE("ArraySelectionIssue: conversion rejects unsupported kinds without throwing", "[ArraySelectionIssue][regression][preflight]")
{
UnitTest::LoadPlugins();
DataStructure data;
PopulateArrays(data);
ConvertDataFilter filter;
auto args = filter.getDefaultArguments();
DataPath path;
SECTION("StringArray")
{
path = DataPath({"Strings"});
}
SECTION("NeighborList")
{
path = DataPath({"IntNeighbors"});
}
args.insertOrAssign(ConvertDataFilter::k_ArrayToConvertPath_Key, std::make_any<DataPath>(path));
IFilter::PreflightResult result;
REQUIRE_NOTHROW(result = filter.preflight(data, args));
REQUIRE(result.outputActions.invalid());
}
Proposed Fix
Give numeric-only consumers a way to require IDataArray before typed access, while preserving explicit support for StringArray and NeighborList in generic consumers. Review ArraySelectionParameter::validatePath, its public configuration, and numeric consumers such as ConvertDataFilter::preflightImpl.
The API choice remains open: explicit allowed array kinds, a numeric-only parameter, or filter-local checks. Define how numeric AllowedTypes, required component shapes, and location restrictions apply to each supported array kind before changing the shared defaults. Audit other unchecked typed preflight accessors and retain regression tests for legitimate generic consumers.
Notes
- Discovered while reviewing simplnx #1817. That PR has a local FeatureIds guard for Compute Surface Area to Volume & Sphericity; it does not fix the shared parameter contract.
- Closed issue #555 records earlier StringArray-selection requirements. That compatibility history matters when choosing a fix.
- Open PR #1766 improves filter type/shape declarations. Its inspected changes do not modify ArraySelectionParameter or ConvertDataFilter, so those declarations do not address this non-IDataArray path.
- Searches of existing issues and PRs found no exact duplicate of this numeric-consumer preflight failure.
Activity