fix #351: Solution 1: enforce weights.shape = x.shape for tuple axis #953
Conversation
This file contains 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
…ape for tuple axis
|
GPU cluster tests are currently disabled on this Pull Request. |
👇 Click on the image for a new way to code review
Legend |
ClaudiaComito
requested changes
Apr 25, 2022
There was a problem hiding this comment.
@Mystic-Slice thanks a lot for taking this on and welcome to our legacy code 😬
I vaguely remember implementing ht.average, it seems so overly complicated now.
I agree with your solution 1, we can enforce - just like numpy - that weights.shape == a.shape if averaging along more than one axis. They must be distributed along the same split axis as well.
The distributed operation itself can be simplified quite a bit, as it's just:
(a*weights).sum(axis)/weights.sum(axis)
a*weights calls _operations.__binary_op() and ht.sum() calls _operations.__reduce_op(), those will take care of sanitizing, communication when needed, etc.
Please go ahead and make the changes whenever you feel comfortable. Thanks a lot!
|
Hey @ClaudiaComito! |
* wip: Initial release draft and changelog updater actions configuration * doc: pr title style guide in contibuting.md * ci: improved release draft templates * ci: extra release draft categories * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci Co-authored-by: Claudia Comito <39374113+ClaudiaComito@users.noreply.github.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
* doc: parallel tutorial note metioning local and global printing * doc: extenden local print note with ``ht.local_printing()`` * Fix typo * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci Co-authored-by: Claudia Comito <39374113+ClaudiaComito@users.noreply.github.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
* Updated the tutorial document. 1. Corrected the spelling mistake -> (sigular to single) 2. Corrected the statement -> the number of dimensions is the rank of the array. 3. Made 2 more small changes. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Fix typo Co-authored-by: Claudia Comito <39374113+ClaudiaComito@users.noreply.github.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
updates: - [github.com/psf/black: 22.3.0 → 22.6.0](psf/black@22.3.0...22.6.0)
…eatures/994-linspace-zero-samples Non-negative sample size for `linspace`/`logspace`
…ug/996-iinfo-finfo-min Bug/996 `iinfo`/`finfo` `min` value
…re-commit-ci-update-config [pre-commit.ci] pre-commit autoupdate
…alytics#1014) * Test latest pyorch on both main and release branch * Move pytorch release record out of workflows directory * Update paths * New PyTorch release * Temporarily remove trigger * Update pytorch-latest.txt * Reinstate trigger * New PyTorch release * Remove matrix strategy * Update pytorch-latest.txt * New PyTorch release * New PyTorch release * fix: set cuda rng state on gpu tests for test_random.py * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Added tests for python 3.9 and pytorch 1.12 Co-authored-by: Claudia Comito <c.comito@fz-juelich.de> Co-authored-by: Daniel Coquelin <daniel.coquelin@gmail.com> Co-authored-by: ClaudiaComito <c.comito@fz-juelich.de@users.noreply.github.com> Co-authored-by: Claudia Comito <39374113+ClaudiaComito@users.noreply.github.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
updates: - [github.com/psf/black: 22.6.0 → 22.8.0](psf/black@22.6.0...22.8.0) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Claudia Comito <39374113+ClaudiaComito@users.noreply.github.com>
fixes formatting issues
fixes an issue where the bug label is not set.
Use status badge from a different workflow action
* Check for split in `__reduce_op` * Check whether x is distributed Co-authored-by: mtar <m.tarnawa@fz-juelich.de> Co-authored-by: mtar <m.tarnawa@fz-juelich.de> Co-authored-by: Claudia Comito <39374113+ClaudiaComito@users.noreply.github.com>
* Update ci worflow action * Update codecov.yml
* Fix `all` * Fix `any` * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Add distributed tests * Expanded tests for combination of axis/split axis Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Claudia Comito <39374113+ClaudiaComito@users.noreply.github.com> Co-authored-by: mtar <m.tarnawa@fz-juelich.de>
updates: - [github.com/psf/black: 22.8.0 → 22.10.0](psf/black@22.8.0...22.10.0)
…pre-commit-ci-update-config [pre-commit.ci] pre-commit autoupdate
|
Changes moved to a new branch. See, #1037 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Issue/s resolved: #351
Type of change
Documentation update
Due Diligence
Does this change modify the behaviour of other functions? If so, which?
yes / no
skip ci