Skip to content

Update sphere.py -- add pooling and max_pooling#1596

Merged
teytaud merged 17 commits intofacebookresearch:mainfrom
mzameshina:main
Feb 20, 2024
Merged

Update sphere.py -- add pooling and max_pooling#1596
teytaud merged 17 commits intofacebookresearch:mainfrom
mzameshina:main

Conversation

@mzameshina
Copy link
Copy Markdown
Contributor

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Motivation and Context / Related issue

How Has This Been Tested (if it applies)

Checklist

  • The documentation is up-to-date with the changes I made.
  • I have read the CONTRIBUTING document and completed the CLA (see CLA).
  • All tests passed, and additional code has been covered with new tests.

@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Feb 16, 2024
Copy link
Copy Markdown
Contributor

@teytaud teytaud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use the same signature as other codes so that it can work in the quasi-randomize function.

Please add your code as default in quasi-randomize, when len(shape) > 1, instead of dispersion_with_big_conv.

"Riesz_blursum_lowconv_midorder",
"Riesz_blursum_lowconv_highorder",
"max_pooling",
"pooling"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All methods in the list have the same signature, so that they can be used by quasi-randomize.
Please use the same signature.

fix signature of pooling and max_pooling
fix style
@teytaud
Copy link
Copy Markdown
Contributor

teytaud commented Feb 18, 2024

Does it run ? This does not work as Torch is not in the dependencies.
I recommend not using torch, as it would imply a big dependency.

Also you need black.

@teytaud teytaud merged commit 75abae1 into facebookresearch:main Feb 20, 2024
teytaud pushed a commit that referenced this pull request Feb 16, 2025
* Update sphere.py

* Update sphere.py

fix signature of pooling and max_pooling

* Update sphere.py

fix style

* Update sphere.py

remove torch dependency

* Update sphere.py

* Update sphere.py

* Update sphere.py

black formatting

* Update sphere.py

* Update sphere.py

- black 
- change standard avg pooling size

* Update sphere.py

* Update sphere.py

* Update sphere.py

fix black

* Update sphere.py

* Update sphere.py

* Update sphere.py

* Update sphere.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed Do not delete this pull request or issue due to inactivity.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants