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

Tweak CCI config #603

Merged
merged 4 commits into from May 2, 2020
Merged

Tweak CCI config #603

merged 4 commits into from May 2, 2020

Conversation

mthrok
Copy link
Collaborator

@mthrok mthrok commented May 1, 2020

Importing the CCI tweak I found useful while working on pytorch/text#748 pytorch/text#744

  • bust conda cache weekly
  • rename utility directory test -> utility
  • Show slowest 20 tests in log --durations 20

@mthrok mthrok requested a review from vincentqb May 1, 2020 21:51
@vincentqb
Copy link
Contributor

The cache is for conda install packages, right?

@vincentqb
Copy link
Contributor

Should the name of the file be changed as well?
.circleci/test/test_sort_yaml.py → .circleci/utils/test_sort_yaml.py

Copy link
Contributor

@vincentqb vincentqb left a comment

Choose a reason for hiding this comment

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

Are you planning to keep the config files in sync between domains?

Copy link
Contributor

@vincentqb vincentqb left a comment

Choose a reason for hiding this comment

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

Oops, I meant to say: tests are failling :/

@mthrok
Copy link
Collaborator Author

mthrok commented May 1, 2020

The cache is for conda install packages, right?

Yes

Should the name of the file be changed as well?
.circleci/test/test_sort_yaml.py.circleci/utils/test_sort_yaml.py

The thing is, I do not know what this script is for and I think it's obsolete. If we care as much I suggest we delete it.

Are you planning to keep the config files in sync between domains?

I don't think that's possible. Torchtext has specific caching which does not apply to other domains.
But these changes felt "good practice to have" in torchaudio too.

Oops, I meant to say: tests are failling :/

Weird, rebuilding test image and will retry.

@vincentqb
Copy link
Contributor

The thing is, I do not know what this script is for and I think it's obsolete. If we care as much I suggest we delete it.

How about we don't modify it as part of this PR, and we simply delete it in the next? :) If we don't see it used anywhere, the risk is low.

Copy link
Contributor

@vincentqb vincentqb left a comment

Choose a reason for hiding this comment

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

LGTM

@vincentqb vincentqb merged commit be18755 into pytorch:master May 2, 2020
@mthrok mthrok deleted the tweak-cci branch May 4, 2020 09:38
mthrok pushed a commit to mthrok/audio that referenced this pull request Feb 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants