Feature/920: Implemented 2D convolution

[shahpratham]

Description

Implemented 2D convolution with distributed kernel support.

Issue resolved: #920

skip ci

[ghost]

👇 Click on the image for a new way to code review

Legend

[ClaudiaComito]

Is this still valid?

[shahpratham]

yes, when a is of dimension (4, 15) and v is of dimension (5, 10)

[krajsek]

Maybe a more precise Error message: "Filter size must not be larger in one dimension and smaller in the other" as the user might not be aware of the swapping in case of the filter size beeing larger in both dimensions

[github-actions]

This pull request is stale because it has been open for 60 days with no activity.

[github-actions]

Thank you for the PR!

[ClaudiaComito]

I cannot reproduce the CUDA problems locally, but even on CPU I run into different problems with mismatching split axes (and local shapes):

======================================================================
======================================================================
ERROR: test_convolve2d (heat.core.tests.test_signal.TestSignal)
ERROR: test_convolve2d (heat.core.tests.test_signal.TestSignal)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/c.comito/devel/local/heat/heat/core/tests/test_signal.py", line 226, in test_convolve2d
    conv = ht.convolve2d(signal, dis_kernel_odd, mode=mode)
  File "/Users/c.comito/devel/local/heat/heat/core/signal.py", line 635, in convolve2d
    signal_filtered += global_signal_filtered[start_idx : start_idx + gshape[0]]
  File "/Users/c.comito/devel/local/heat/heat/core/arithmetics.py", line 188, in add_
    return _operations.__binary_op(wrap_add_, t1, t2, out=t1)
  File "/Users/c.comito/devel/local/heat/heat/core/_operations.py", line 197, in __binary_op
    sanitation.sanitize_out(out, output_shape, output_split, output_device, output_comm)
  File "/Users/c.comito/devel/local/heat/heat/core/sanitation.py", line 300, in sanitize_out
    raise ValueError(f"Expecting output buffer of split {output_split}, got {out.split}")
ValueError: Expecting output buffer of split 0, got None

(tests run on 2 processes).

[ClaudiaComito]

I'm setting this PR to draft and marking it for next Monday's discussion. My thoughts so far (fed from discussions with @mrfh92 in the past):

• code still needs significant debugging
• fully distributed (signal and kernel) scaling behaviour unclear
• fully distributed via FFT is being implemented (Distributed Convolution via FFT #1291 )

Options for this PR:

1. fix bugs and merge independent of scaling behaviour (needs work)
2. throw out distributed-kernel implementation, keep distributed-signal option only (needs work but will cover most use cases)
3. do not merge
4. ...??

What do you think @mrfh92 @krajsek @mtar ?

[github-actions]

This pull request is stale because it has been open for 60 days with no activity.

[github-actions]

This pull request was closed because it has been inactive for 60 days since being marked as stale.

[JuanPedroGHM]

Work will continue in #1937, closing this one.