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.

Sign up for GitHub

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

Jump to bottom

Define a dataclass for parameter metadata. #2218

Merged
merged 3 commits into from
Jun 30, 2022
Merged

Define a dataclass for parameter metadata. #2218

merged 3 commits into from
Jun 30, 2022

Conversation

justinxzhao
Copy link
Contributor

No description provided.

@justinxzhao justinxzhao marked this pull request as ready for review June 30, 2022 05:28
@github-actions
Copy link

Unit Test Results

       6 files  ±0         6 suites  ±0   2h 22m 9s ⏱️ - 3m 8s
2 908 tests ±0  2 862 ✔️ ±0    46 💤 ±0  0 ❌ ±0 
8 724 runs  ±0  8 582 ✔️ ±0  142 💤 ±0  0 ❌ ±0 

Results for commit 0839c5d. ± Comparison against base commit 5df979f.

ksbrar
ksbrar approved these changes Jun 30, 2022
View reviewed changes
Copy link
Collaborator

@ksbrar ksbrar left a comment

Choose a reason for hiding this comment

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

Smaller PRs are indeed better 😅

Two Qs.

ludwig/schema/parameter_metadata.py Show resolved Hide resolved
ludwig/schema/parameter_metadata.py
- LOW: this parameter usually does not have a significant impact on model performance.
"""

UNKNOWN = 0
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is the rationale for having an UNKNOWN flag? I would assume default as LOW

Copy link
Contributor Author

Choose a reason for hiding this comment

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

UNKNOWN allows us to distinguish parameters that haven't been annotated vs. parameters that have been explicitly marked as having LOW impact.

@justinxzhao justinxzhao merged commit f13f45e into master Jun 30, 2022
@justinxzhao justinxzhao deleted the parameter_metadata branch June 30, 2022 15:19
@ksbrar ksbrar mentioned this pull request Jun 30, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ksbrar ksbrar ksbrar approved these changes

@connor-mccorm connor-mccorm Awaiting requested review from connor-mccorm

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@justinxzhao @ksbrar