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

Set minimum pytorch version to 1.8 + cleanup #1263

Merged
merged 13 commits into from
Oct 12, 2022
Merged

Set minimum pytorch version to 1.8 + cleanup #1263

merged 13 commits into from
Oct 12, 2022

Conversation

SkafteNicki
Copy link
Member

@SkafteNicki SkafteNicki commented Oct 11, 2022

What does this PR do?

Fixes #1262
Sets minimum pytorch version to 1.8 and clean-up all skipping/conditioning throughout the code that was necessary to run for older versions.

Before submitting

  • Was this discussed/approved via a Github issue? (no need for typos and docs improvements)
  • Did you read the contributor guideline, Pull Request section?
  • Did you make sure to update the docs?
  • Did you write any new necessary tests?

PR review

Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.

Did you have fun?

Make sure you had fun coding 🙃

@SkafteNicki SkafteNicki added the refactoring refactoring and code health label Oct 11, 2022
@SkafteNicki SkafteNicki added this to the v0.11 milestone Oct 11, 2022
@stancld
Copy link
Contributor

stancld commented Oct 11, 2022

#1259 has set v0.10 milestone (maybe for a possible patch release). We should make this in sync.

@codecov
Copy link

codecov bot commented Oct 11, 2022

Codecov Report

Merging #1263 (df0523f) into master (6ae08a8) will decrease coverage by 0%.
The diff coverage is 96%.

Additional details and impacted files
@@          Coverage Diff           @@
##           master   #1263   +/-   ##
======================================
- Coverage      86%     86%   -0%     
======================================
  Files         195     195           
  Lines       11463   11388   -75     
======================================
- Hits         9836    9762   -74     
+ Misses       1627    1626    -1     

requirements.txt Outdated Show resolved Hide resolved
@Borda Borda enabled auto-merge (squash) October 11, 2022 13:47
@SkafteNicki
Copy link
Member Author

#1259 has set v0.10 milestone (maybe for a possible patch release). We should make this in sync.

You are right. But then we probably need to move #1259 to v0.11 because this kind of breaking change to minimum requirement i think we need to do as an v0.x.0 release and not v0.10.x.

@SkafteNicki
Copy link
Member Author

seems like CI pulling the wrong version of torch for oldest config. Maybe the cache is needs to be reset?

@stancld
Copy link
Contributor

stancld commented Oct 11, 2022

@SkafteNicki @Borda There's a .github/assistant.py file containing the following dict:

LUT_PYTHON_TORCH = {
    "3.8": "1.4",
    "3.9": "1.7.1",
    "3.10": "1.11",
}

I guess this has to be updated

@Borda
Copy link
Member

Borda commented Oct 11, 2022

There's a .github/assistant.py file containing the following dict:

seems like a bug in the assistant as it shall take the max

@Borda Borda disabled auto-merge October 11, 2022 20:06
@Borda Borda enabled auto-merge (squash) October 11, 2022 20:06
@Borda
Copy link
Member

Borda commented Oct 11, 2022

@SkafteNicki seems there are a few tests failing for Windows and PT 1.8

@mergify mergify bot added the ready label Oct 12, 2022
@Borda Borda merged commit 173e88e into master Oct 12, 2022
@Borda Borda deleted the min_pt_1_8 branch October 12, 2022 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready refactoring refactoring and code health
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update minimum requirement of torch to 1.8
4 participants