Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update pad.py to include reflective padding #195

Closed
wants to merge 33 commits into from
Closed

Conversation

lmanan
Copy link

@lmanan lmanan commented Nov 2, 2023

Pre-Merge Checklist:

  • if this PR adds a feature: request merge into the latest development branch (vX.Y-dev)
  • if this PR fixes a bug: request merge into the latest patch branch (patch-X.Y.Z)
  • PR branch name is short and describes the feature/bug addressed in this PR
  • commit messages are consistent
  • changes reviewed by another contributor
  • tests cover changes
  • all tests pass

This PR adds capability for enabling reflection-padding in pad.py here.

pattonw and others added 15 commits October 6, 2023 13:37
fix documentation to be more accurate around nonspatial arrays
allow None roi/voxel size for spatial arrays
no longer assumes a deformed label will still exist in an array
This is to get around local functions (i.e. the hooks) not being
pickle-able which we need for the "spawn" start function
(spawn is the default on windows and recent macs)
We want to test with both fork and spawn start methods, but this
seems to interfere with the torch tests
@lmanan lmanan requested a review from funkey November 2, 2023 15:59
@pattonw
Copy link
Collaborator

pattonw commented Nov 2, 2023

Can you add tests?
What happens if the padded region is larger than the contained region along some axis?

@pattonw
Copy link
Collaborator

pattonw commented Nov 2, 2023

Also please target the patch-1.3.2 branch for your pull request

@codecov-commenter
Copy link

codecov-commenter commented Nov 3, 2023

Codecov Report

Merging #195 (d721190) into main (a33e5b4) will decrease coverage by 0.12%.
The diff coverage is 27.77%.

@@            Coverage Diff             @@
##             main     #195      +/-   ##
==========================================
- Coverage   67.60%   67.48%   -0.12%     
==========================================
  Files          98       98              
  Lines        6829     6844      +15     
==========================================
+ Hits         4617     4619       +2     
- Misses       2212     2225      +13     
Files Coverage Δ
gunpowder/nodes/pad.py 76.00% <27.77%> (-15.67%) ⬇️

if using start method = "spawn" and the "start_subprocess" flag
for the predict node, we now pass our test.
if using the start method "spawn", and the "spawn_subprocess" flag for
the train node, we now pass our test
Now prints the errors in reverse order of execution so the initial pipeline error is printed first
pattonw and others added 5 commits December 19, 2023 08:45
commit 1686b949766b76960534ede1105751591fd91c9f
Author: William Patton <[email protected]>
Date:   Tue Dec 19 08:43:11 2023 -0700

    black reformatting

commit 26d2c7cfff3f2702f56a5bb4249a0811f54b45ef
Author: Mohinta2892 <[email protected]>
Date:   Thu Nov 2 19:09:15 2023 +0000

    Revert "black reformatted"

    This reverts commit 66dd69b.

    Only format changed files, since black does not consider formatting history

commit a273fd3813fc16b516c2438ad5af0c4ee3f0686b
Author: Samia Mohinta <[email protected]>
Date:   Thu Nov 2 17:12:26 2023 +0000

    black reformatted

commit bb37769eec33af5921386f283e2579055bb34e6d
Author: Samia Mohinta <[email protected]>
Date:   Thu Nov 2 16:40:32 2023 +0000

    add device arg

    Allow passing cuda device to Predict. Issue #188

commit a3b3588a1406d609ae95370cf2c5339872616011
Author: Samia Mohinta <[email protected]>
Date:   Thu Nov 2 16:39:09 2023 +0000

    add device arg

    allow passing cuda device to Train
currently failing a few of them, some are expected failures.
parametrized the use of constant or reflect padding.

Now avoids using the unittest framework
@pattonw
Copy link
Collaborator

pattonw commented Dec 19, 2023

closing in favor of #197
That pull request targets the patch branch and includes tests

@pattonw pattonw closed this Dec 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants