From bfe670ebba1a73d41275a7f934aff130bbb914bb Mon Sep 17 00:00:00 2001 From: Steven Atkinson Date: Wed, 11 Feb 2026 18:29:30 -0800 Subject: [PATCH 1/3] Make activation apply method pure virtual instead of no-op default --- NAM/activations.h | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/NAM/activations.h b/NAM/activations.h index 68d5025..3982e14 100644 --- a/NAM/activations.h +++ b/NAM/activations.h @@ -150,7 +150,7 @@ class Activation { apply(block.data(), block.rows() * block.cols()); } - virtual void apply(float* data, long size) {} + virtual void apply(float* data, long size) = 0; static Ptr get_activation(const std::string name); static Ptr get_activation(const ActivationConfig& config); @@ -165,13 +165,13 @@ class Activation static std::unordered_map _activations; }; -// identity function activation +// identity function activation--"do nothing" class ActivationIdentity : public nam::activations::Activation { public: ActivationIdentity() = default; ~ActivationIdentity() = default; - // Inherit the default apply methods which do nothing + virtual void apply(float* data, long size) override {}; }; class ActivationTanh : public Activation @@ -285,7 +285,9 @@ class ActivationPReLU : public Activation std::vector slopes_for_channels = negative_slopes; // Fail loudly if input has more channels than activation - assert(actual_channels == negative_slopes.size()); + assert(actual_channels == negative_slopes.size() + && ("Received input with " + std::to_string(actual_channels) + " channels, but activation has " + + std::to_string(negative_slopes.size()) + " channels")); // Apply each negative slope to its corresponding channel for (unsigned long channel = 0; channel < actual_channels; channel++) From c2bc28854b88fc2c6e0ca1e1a500ecb401dddad9 Mon Sep 17 00:00:00 2001 From: Steven Atkinson Date: Wed, 11 Feb 2026 18:56:08 -0800 Subject: [PATCH 2/3] Fix bugs --- NAM/activations.h | 31 ++++++++++++++++++++++++++++--- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/NAM/activations.h b/NAM/activations.h index 3982e14..59fcb90 100644 --- a/NAM/activations.h +++ b/NAM/activations.h @@ -2,6 +2,7 @@ #include #include // expf +#include // std::cerr #include #include #include @@ -276,6 +277,25 @@ class ActivationPReLU : public Activation } ActivationPReLU(std::vector ns) { negative_slopes = ns; } + void apply(float* data, long size) override + { + // Assume column-major (this is brittle) +#ifndef NDEBUG + if (size % negative_slopes.size() != 0) + { + std::cerr << "PReLU.apply(*data, size) was given an array of size " << size + << " but the activation has " << negative_slopes.size() + << " channels, which doesn't divide evenly.\n"; + assert(false); + } +#endif + for (long pos = 0; pos < size; pos++) + { + const float negative_slope = negative_slopes[pos % negative_slopes.size()]; + data[pos] = leaky_relu(data[pos], negative_slope); + } + } + void apply(Eigen::MatrixXf& matrix) override { // Matrix is organized as (channels, time_steps) @@ -285,9 +305,14 @@ class ActivationPReLU : public Activation std::vector slopes_for_channels = negative_slopes; // Fail loudly if input has more channels than activation - assert(actual_channels == negative_slopes.size() - && ("Received input with " + std::to_string(actual_channels) + " channels, but activation has " - + std::to_string(negative_slopes.size()) + " channels")); +#ifndef NDEBUG + if (actual_channels != negative_slopes.size()) + { + std::cerr << "PReLU: Received " << actual_channels << " channels, but activation has " + << negative_slopes.size() << " channels\n"; + assert(false); + } +#endif // Apply each negative slope to its corresponding channel for (unsigned long channel = 0; channel < actual_channels; channel++) From e191afb5fb3daa6db5778d4b9ac2c71f5756b9d2 Mon Sep 17 00:00:00 2001 From: Steven Atkinson Date: Wed, 11 Feb 2026 19:05:23 -0800 Subject: [PATCH 3/3] Refactor to throw std::invalid_argument in debug mode, add tests --- NAM/activations.h | 16 ++++----- tools/run_tests.cpp | 5 +-- tools/test/test_activations.cpp | 61 +++++++++++++++++++++++++++++---- 3 files changed, 66 insertions(+), 16 deletions(-) diff --git a/NAM/activations.h b/NAM/activations.h index 59fcb90..a05c456 100644 --- a/NAM/activations.h +++ b/NAM/activations.h @@ -2,7 +2,8 @@ #include #include // expf -#include // std::cerr +#include // std::cerr (kept for potential debug use) +#include // std::invalid_argument #include #include #include @@ -283,10 +284,9 @@ class ActivationPReLU : public Activation #ifndef NDEBUG if (size % negative_slopes.size() != 0) { - std::cerr << "PReLU.apply(*data, size) was given an array of size " << size - << " but the activation has " << negative_slopes.size() - << " channels, which doesn't divide evenly.\n"; - assert(false); + throw std::invalid_argument("PReLU.apply(*data, size) was given an array of size " + std::to_string(size) + + " but the activation has " + std::to_string(negative_slopes.size()) + + " channels, which doesn't divide evenly."); } #endif for (long pos = 0; pos < size; pos++) @@ -308,9 +308,9 @@ class ActivationPReLU : public Activation #ifndef NDEBUG if (actual_channels != negative_slopes.size()) { - std::cerr << "PReLU: Received " << actual_channels << " channels, but activation has " - << negative_slopes.size() << " channels\n"; - assert(false); + throw std::invalid_argument("PReLU: Received " + std::to_string(actual_channels) + + " channels, but activation has " + std::to_string(negative_slopes.size()) + + " channels"); } #endif diff --git a/tools/run_tests.cpp b/tools/run_tests.cpp index 9b3bdec..1c8c34a 100644 --- a/tools/run_tests.cpp +++ b/tools/run_tests.cpp @@ -48,8 +48,9 @@ int main() test_activations::TestPReLU::test_core_function(); test_activations::TestPReLU::test_per_channel_behavior(); - // This is enforced by an assert so it doesn't need to be tested - // test_activations::TestPReLU::test_wrong_number_of_channels(); + test_activations::TestPReLU::test_wrong_number_of_channels_matrix(); + test_activations::TestPReLU::test_wrong_size_array(); + test_activations::TestPReLU::test_valid_array_size(); // Typed ActivationConfig tests test_activations::TestTypedActivationConfig::test_simple_config(); diff --git a/tools/test/test_activations.cpp b/tools/test/test_activations.cpp index a8dd705..55d61ff 100644 --- a/tools/test/test_activations.cpp +++ b/tools/test/test_activations.cpp @@ -220,9 +220,10 @@ class TestPReLU assert(fabs(data(1, 2) - 0.0f) < 1e-6); // 0.0 (unchanged) } - static void test_wrong_number_of_channels() + static void test_wrong_number_of_channels_matrix() { - // Test that we fail when we have more channels than slopes + // Test that we fail when matrix has more channels than slopes + // Note: This validation only runs in debug builds (#ifndef NDEBUG) Eigen::MatrixXf data(3, 2); // 3 channels, 2 time steps // Initialize with test data @@ -232,21 +233,69 @@ class TestPReLU std::vector slopes = {0.01f, 0.05f}; nam::activations::ActivationPReLU prelu(slopes); - // Apply the activation +#ifndef NDEBUG + // In debug mode, this should throw std::invalid_argument bool caught = false; try { prelu.apply(data); } - catch (const std::runtime_error& e) + catch (const std::invalid_argument& e) { caught = true; } - catch (...) + assert(caught && "Expected std::invalid_argument for channel count mismatch"); +#endif + } + + static void test_wrong_size_array() + { + // Test that we fail when array size doesn't divide evenly by channel count + // Note: This validation only runs in debug builds (#ifndef NDEBUG) + + // Create PReLU with 2 channels + std::vector slopes = {0.01f, 0.05f}; + nam::activations::ActivationPReLU prelu(slopes); + + // Array of size 5 doesn't divide evenly by 2 channels + std::vector data = {-1.0f, -2.0f, 0.5f, 1.0f, -0.5f}; + +#ifndef NDEBUG + // In debug mode, this should throw std::invalid_argument + bool caught = false; + try + { + prelu.apply(data.data(), (long)data.size()); + } + catch (const std::invalid_argument& e) { + caught = true; } + assert(caught && "Expected std::invalid_argument for array size mismatch"); +#endif + } + + static void test_valid_array_size() + { + // Test that valid array sizes work correctly + + // Create PReLU with 2 channels + std::vector slopes = {0.1f, 0.2f}; + nam::activations::ActivationPReLU prelu(slopes); + + // Array of size 6 divides evenly by 2 channels (3 time steps per channel) + std::vector data = {-1.0f, -1.0f, -1.0f, -1.0f, -1.0f, -1.0f}; + + // Should not throw + prelu.apply(data.data(), (long)data.size()); - assert(caught); + // Verify results: alternating between slope 0.1 and 0.2 + assert(fabs(data[0] - (-0.1f)) < 1e-6); // channel 0, slope 0.1 + assert(fabs(data[1] - (-0.2f)) < 1e-6); // channel 1, slope 0.2 + assert(fabs(data[2] - (-0.1f)) < 1e-6); // channel 0, slope 0.1 + assert(fabs(data[3] - (-0.2f)) < 1e-6); // channel 1, slope 0.2 + assert(fabs(data[4] - (-0.1f)) < 1e-6); // channel 0, slope 0.1 + assert(fabs(data[5] - (-0.2f)) < 1e-6); // channel 1, slope 0.2 } };