fix PoW and other functions not working with USE_TORCH=0 despite torch being available #1917
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
mjurbanski-reef
force-pushed
the
improve_no_torch
branch
2 times, most recently
from
85cdc00
to
db3ef49
Compare
mjurbanski-reef
commented
May 22, 2024
|
|
||
| U32_MAX = 4294967295 | ||
| U16_MAX = 65535 | ||
|
|
||
|
|
||
| @legacy_torch_api_compat |
There was a problem hiding this comment.
IMO this approach of just having one implementation of a function and then translating only input and output will be much easier to maintain, but I leave it to others if they want to use it in more places or if they want to keep using if use_torch() all over the place
…h being available
mjurbanski-reef
force-pushed
the
improve_no_torch
branch
from
db3ef49
to
bb758b3
Compare
|
@thewhaleking as author of #1904 you maybe interested in this PR @gus-opentensor this probably should also land in v7, as it will make users less reluctant to switch to numpy interface |
bittensor/utils/registration.py
Outdated
| def __init__(self): | ||
| self._transformed = False | ||
| def legacy_torch_api_compat(func): | ||
| """Decorator to convert numpy arrays to torch tensors before passing them to the function. |
There was a problem hiding this comment.
I really like this approach. I would recommend updating this line, as it reads as the opposite of what it's saying. Other than that, this seems great.
thewhaleking
approved these changes
May 22, 2024
This was referenced May 22, 2024
ibraheem-opentensor
pushed a commit
that referenced
this pull request
May 24, 2024
fix PoW and other functions not working with USE_TORCH=0 despite torch being available
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.
Bug
This fixes problem that after #1904 uses wanting PoW and other torch-using functionality were forced to use USE_TORCH=1 which also forced the legacy torch-based interface.
Description of the Change
USE_TORCH=0, so users don't accidentally run legacy interface - we want old users to move to new one as soon as it is viable AND none of the new users should accidentally run legacy interfacelegacy_torch_api_compatfor an easier way of maintaining two interfaces, but only used it in limited fashion for nowAlternate Designs
as written above, no
Possible Drawbacks
No real drawbacks that Im aware of.
Verification Process
Unit tests added and manual tests.
Release Notes
None, this fixes regression introduced by #1904 that was yet to be released.