2D Convolution #1937
Open
2D Convolution #1937
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
for more information, see https://pre-commit.ci
… had to replace ht.int by ht.float32
for more information, see https://pre-commit.ci
- instead use str(ht_array.device) - Use signal.device to get correct rank
- put more functionality into input_check - add batch processing check - add stride to convolution 2D - fix convgenpad regarding circular condition - prepare convolution2D for batch-processing (not yet implemented) - Improve indexing in convolution2D for arbitrary dimensions test_signal.py - Start adding second dimension to test_only_balanced_kernels
Split up 1D tests into subtests - test conv_input_check - test conv_batchprocessing_check - test batch_cnvolutions 1D with and without stride
- test 1D batch processing with and without stride - remove batchprocessing test from test_convolve into own function - 2D batch processing tests, test NotImplementedError
- Test odd and even kernels separately - With and without stride
- test large signal and kernel for different modes with/without stride - test kernel size 1 with/without stride - remove test_convolve
lolacaro
force-pushed
the
feature920/distributed-2D-convolution
branch
from
d8359df to
dd0f899
Compare
- Tests for different modes - Tests for different kernels - Tests for different strides - Different formats - Edge cases
- Feature still missing: Batch convolutions for 2D
Open question: How to handle distributed arrays in conv_pad if the rank is empty? Or rather not pad, if empty However, in this case: the last non-empty rank is the reference for padding
side note: Maybe add a RuntimeError or other if boundary == "replicate" and split dimension is 1 and any rank is zero! Will throw an error from torch
- Still broken: Local-index computation for even kernels Fix: - correct array shapes for DND result - 2D convolutions, with stride 1 for distributed kernels
convolution2d_large_signal_and_kernel still throws error for distributed kernels
Issue: Flip of kernel did not occur along split dimension Solution: Use positive indexing - Added scripts/flip_text.py to demonstrate problem - Adjusted signal.py to positive indexing Open problems: convolve2d with distributed kernel still fails, likely due to broadcasting issue
- kernel chunks were not distributed by v.comm.bcast - Switch to v.comm.Bcast Additional fix: line 915 - check a.isdistributed() before calling stride[a.split] All tests pass but one: - test_convolve2d_local_chunks_error
Problem: chunk error test after padding, but now also after halo computation Solution: - Adjust test to comm sizes > 3, because otherwise no problem arises - To my knowledge, the chunk test is also valid after halo compute, so no additional fix needed
- communication in _allgather fails - only appeared for mpirun -n >3 and convolve2d with stride and v.is_distributed() - problem likely relates to issues within convolve2d where ranks loos the information despite axis=None
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Copy of #1007 due to branching conflicts
Due Diligence
Description
Issue/s resolved: #
Changes proposed:
Type of change
Memory requirements
Performance
Does this change modify the behaviour of other functions? If so, which?
yes / no