Implement Default fmod Functionality for Base Tensor Type #46#280
Conversation
nahuakang
changed the title
|
Thank you @nahuakang! Can you solve first the style issues that Travis reported? (you can reproduce them locally with Also, I think that PR #219 is also trying to tackle #46 but I'm not sure about its status. (ping: @radicalrafi) |
|
PR #219 is supposed to tackle this one will submit final edits today . |
|
@aiorla Thank you for your feedback. There seems to be a lot of Flake8 errors from 1113-1145. Is this caused by some actions from me? And is the best way to fix style issues just to do it manually? @radicalrafi Thanks. So I should stop working on this issue now? |
|
P.S I'm running in some trouble with my local env can't seem to fix if yours is good to go I have closed my OLD PR #219 |
|
You might need to run flake8 locally or trace the Travis Log and fix the syntax issues . |
|
@radicalrafi @aiorla Thank you both for your help, and I manually fixed the line and space issues. Travis-CI builds successfully now. |
|
@nahuakang this looks great, could you please rewrite this in math.py and add the respective methods in tensor.py? (sorry to ask these changes now! 😬) but otherwise we are good to go. |
|
@bharathgs If I'm understanding you correctly, your advice is:
|
|
@bharathgs I won't comment on the function location because I'm still not sure about what rules are we following. (but you may be right) My feedback about the PR (I was waiting Travis to check it):
PS: I think PyTorch is a bit inconsistent in its definition. It's not clear if we need the same shape or just for them to be broadcastable... 😟 |
|
@aiorla yes you are right (about the feedback) @nahuakang yes you are right as well. |
|
@aiorla @bharathgs |
|
Looks 👍 to me. Thanks for your patience @nahuakang! |
* __init__.py: - Fixed a typo in execute_command(command) - Added documentation to launch_on_heroku function - Removed unessacery if from launch_on_heroku utils.py: - Added documentation * Updated Part1 example notebook * Update __init__.py * Update utils.py * ran black for formatting on grid
* __init__.py: - Fixed a typo in execute_command(command) - Added documentation to launch_on_heroku function - Removed unessacery if from launch_on_heroku utils.py: - Added documentation * Updated Part1 example notebook * Update __init__.py * Update utils.py * ran black for formatting on grid
refactored client and client config
* fix patch upgrades compatible on older versions
* refactor: split versioned folder lookup into reusable primitives
Replace the fused _find_compatible_versioned_folder helper (75 lines,
parse_version callback parameter, four per-format parser functions) with
two small primitives plus an "expect exactly one" helper:
_find_folders -- generic Drive folder lister; returns (id, name)
pairs; knows nothing about versions. Useful
anywhere we list folders.
_filter_patch_compatible
-- walks each name's '#'/'-'-separated chunks for
an X.Y.Z-shaped field, keeps those with matching
major.minor. Format-agnostic.
_expect_one -- 0 -> None, 1 -> id, many -> raise. Used at all
four call sites.
Removes the four per-format parsers and the parse_version= callback. Call
sites become three flat lines each. Behavior preserved: same patch-compat
semantics, same multi-match error.
Bug-fix: _filter_patch_compatible's current_version arg uses a None
sentinel resolved at call time, not a module-level default captured at
import time -- otherwise tests that patch SYFT_CLIENT_VERSION miss.
The personal-folder lookup picks up one inline filter (name.count('#')==1)
because the substring '#{email}' also matches p2p folders ending in the
peer slot; the other three call sites have tight enough name_contains
substrings to discriminate on their own.
Adds tests/unit/test_versioned_folder_lookup.py with 17 pure-function
tests for _looks_like_version, _extract_version_from_name, and
_filter_patch_compatible. Full unit suite: 407 pass.
* chore: nbstripout DO_Tutorial_V3.ipynb to unblock pre-commit
---------
Co-authored-by: bitsofsteve <cyberlexix@gmail.com>
4 participants
Hi y'all,
This is another attempt to try pull request and to ask for feedback. Both
fmod()andfmod()_ methods are added together with corresponding unit tests. However,fmod()is slightly different from the one in PyTorch as it does not contain the optional parameterout.Thank you for your time and feedback!
Best,
Nahua