327 add circularbatch and circulartensor tests and fix a bug detected in cvgpuspeedup

[morousg]

No description provided.

[Copilot]

Pull request overview

Adds new test coverage around circular-batch reads and CircularTensor update behavior, while fixing an out-of-bounds plane-count bug exposed by DivergentBatchTransformDPP usage and extending divergent-batch execution to the CPU backend.

Changes:

• Add new tests for CircularBatchRead, CircularTensor, and DivergentBatchTransformDPP.
• Fix CircularTensor::update() divergent-batch plane accounting by narrowing the copy-sequence plane view and making CircularTensorRead::num_elems_z() respect the provided view.
• Refactor TransformDPP to expose exec_thread() and add a CPU Executor specialization for DivergentBatchTransformDPP.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
tests/operation/test_cricular_batch.h	Adds a test for CircularBatchRead producing correct plane rotation.
tests/data/test_circular_tensor.h	Adds a test that repeatedly updates a CircularTensor and validates ordering.
tests/data_parallel_patterns/test_divergent_batch.h	Adds a divergent-batch test (currently has compile/runtime issues noted in comments).
include/fused_kernel/core/execution_model/executors.h	Adds CPU executor support for DivergentBatchTransformDPP.
include/fused_kernel/core/execution_model/data_parallel_patterns.h	Introduces TransformDPP::exec_thread() and updates divergent-batch to pass full thread coords.
include/fused_kernel/core/data/circular_tensor.h	Narrows copy-sequence plane view to BATCH-1 to fix divergent-batch plane-space sizing.
include/fused_kernel/algorithms/basic_ops/memory_operations.h	Fixes CircularTensorRead::num_elems_z() to reflect the runtime view’s plane count.

Suppressed comments (1)

tests/data_parallel_patterns/test_divergent_batch.h:60

• Same issue as above: x loop bound uses HEIGHT instead of WIDTH, which is incorrect when WIDTH != HEIGHT.

                for (int x = 0; x < HEIGHT; x++) {

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Suppressed comments (8)

tests/data_parallel_patterns/test_divergent_batch.h:22

• This test uses std::vector/std::array/std::cout/std::runtime_error but doesn't include the corresponding standard headers. Depending on transitive includes, this can fail to compile (especially with different libstdc++/MSVC STL implementations). Add explicit includes for portability.

#include <fused_kernel/core/execution_model/data_parallel_patterns.h>
#include <fused_kernel/core/core.h>
#include <fused_kernel/core/data/ptr_utils.h>
#include <fused_kernel/algorithms/basic_ops/arithmetic.h>
#include <fused_kernel/fused_kernel.h>

tests/data_parallel_patterns/test_divergent_batch.h:62

• Ground-truth initialization iterates x < HEIGHT in the z != 0 branch, which is incorrect when WIDTH != HEIGHT and can leave parts of the tensor uninitialized. It should iterate to WIDTH (as done in the z == 0 branch).

            for (int y = 0; y < HEIGHT; y++) {
                for (int x = 0; x < HEIGHT; x++) {
                    const fk::Point p{x, y, z};
                    *fk::PtrAccessor<fk::ND::_3D>::point(p, h_groundTruth.ptr()) = z;

tests/data_parallel_patterns/test_divergent_batch.h:102

• The failure branch throws and then assigns returnValue = -1;, but that assignment is unreachable. Most other tests in this repo report failure via the return code; removing the throw also avoids needing exception support in every build configuration.

    } else {
        std::cout << "testDivergentBatch Failed!" << std::endl;
        throw std::runtime_error("Test failed!");
        returnValue = -1;
    }

include/fused_kernel/core/execution_model/data_parallel_patterns.h:267

• TransformDPP<ParArch::CPU>::exec_thread recomputes activeThreads on every call. Since exec() already computes activeThreads once and then calls exec_thread per element, this adds an extra getActiveThreads() per pixel (and also impacts DivergentBatchTransformDPP's CPU path, which calls exec_thread in an inner loop). Adding an overload that accepts a precomputed ActiveThreads avoids this overhead while preserving the existing API.

        FK_HOST_FUSE void exec_thread(const Point& thread, const Details& details, const IOps&... iOps) {
            const ActiveThreads activeThreads = getActiveThreads(details, get_arg<0>(iOps...));

            if (thread.x < activeThreads.x && thread.y < activeThreads.y) {
                Parent::execute_thread(thread, activeThreads, iOps...);

include/fused_kernel/core/execution_model/data_parallel_patterns.h:305

• On the CPU path, launchTransformDPP already computes activeThreads once, but then calls TDPP::exec_thread(...) which (currently) recomputes activeThreads per pixel. With the exec_thread(thread, activeThreads, ...) overload, this call site should pass the precomputed activeThreads to avoid the per-pixel recomputation.

                const ActiveThreads activeThreads = TDPP::getActiveThreads(Details{}, get_arg<0>(iOps...));
                for (int y = 0; y < static_cast<int>(activeThreads.y); ++y) {
                    for (int x = 0; x < static_cast<int>(activeThreads.x); ++x) {
                        TDPP::exec_thread(Point{ x, y, thread.z }, Details{}, iOps...);
                    }

tests/operation/test_cricular_batch.h:21

• This test uses std::vector and std::array but doesn't include <vector>/<array>. Relying on transitive includes is brittle and can fail on different standard library implementations.

#include <fused_kernel/algorithms/basic_ops/memory_operations.h>
#include <fused_kernel/fused_kernel.h>

#include <iostream>

tests/operation/test_cricular_batch.h:1

• The filename test_cricular_batch.h appears to have a typo (“cricular” vs “circular”). Since test discovery is filename-based, this can make the test harder to find/grep and looks accidental; consider renaming to test_circular_batch.h for clarity.

This issue also appears on line 18 of the same file.

/* Copyright 2023 Mediaproduccion S.L.U. (Oscar Amoros Huguet)

tests/data/test_circular_tensor.h:23

• This file uses typeid(IT).name() / typeid(OT).name() but doesn't include <typeinfo>. Some standard library implementations require the explicit header for std::type_info::name().

#include <iostream>