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

[Bug] Make Configs Consistent #928

Merged
merged 2 commits into from
May 9, 2023

Conversation

austinburnett
Copy link
Contributor

@austinburnett austinburnett commented May 9, 2023

Checklist:

  • I have added a warning that '-' may be deprecated in relevant readme.
  • I have updated all documentation to be consistent. Replacing all occurrences of '-' with '_'
  • I have modified demo files to also be consistent.

@CLAassistant
Copy link

CLAassistant commented May 9, 2023

CLA assistant check
All committers have signed the CLA.

@StellaAthena
Copy link
Member

Why isn't "I have modified demo files to also be consistent." marked as completed? It looks like it has been?

To be clear, that's referring to the files in gpt-neox/configs

@austinburnett
Copy link
Contributor Author

Why isn't "I have modified demo files to also be consistent." marked as completed? It looks like it has been?

To be clear, that's referring to the files in gpt-neox/configs

The reason this wasn't marked as complete was that I was unsure if these were the specific files you were referring to. I'll go ahead and check it off now that its been verified.

@StellaAthena StellaAthena added bug Something isn't working and removed bug Something isn't working labels May 9, 2023
@austinburnett
Copy link
Contributor Author

Looks like some of the checks are failing. I'll review my changes and make sure I didn't replace a '-' with a '_' that should've stayed a '-'.

@StellaAthena
Copy link
Member

Try running pre-commit run --all-files locally

@austinburnett
Copy link
Contributor Author

Try running pre-commit run --all-files locally

Thanks! All checks passed locally for the new commit.

Copy link
Member

@Quentin-Anthony Quentin-Anthony left a comment

Choose a reason for hiding this comment

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

LGTM. I'll deal with the pre-commit checks in a separate PR.

@StellaAthena -- Take a look yourself and merge.

@austinburnett
Copy link
Contributor Author

LGTM. I'll deal with the pre-commit checks in a separate PR.

@StellaAthena -- Take a look yourself and merge.

LGTM!!!! Sounds good. If you don't mind, could you tell me what the issue was when it gets sorted out.... trying to learn as much as possible!

@Quentin-Anthony
Copy link
Member

Quentin-Anthony commented May 9, 2023

LGTM. I'll deal with the pre-commit checks in a separate PR.
@StellaAthena -- Take a look yourself and merge.

LGTM!!!! Sounds good. If you don't mind, could you tell me what the issue was when it gets sorted out.... trying to learn as much as possible!

Yep I'll let you know what it ends up being :)

@austinburnett
Copy link
Contributor Author

austinburnett commented May 9, 2023

LGTM. I'll deal with the pre-commit checks in a separate PR.
@StellaAthena -- Take a look yourself and merge.

LGTM!!!! Sounds good. If you don't mind, could you tell me what the issue was when it gets sorted out.... trying to learn as much as possible!

Yep I'll let you know what it ends up being :)

Thank you!! I'd like to contribute on a programming-related issue shortly. I'll be busy the rest of this week preparing for interviews. If possible, could you point me to a good next issue to work on.

@StellaAthena StellaAthena merged commit 3719533 into EleutherAI:main May 9, 2023
@austinburnett austinburnett deleted the bug/consistentConfigs branch May 10, 2023 02:29
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

4 participants