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

Use concrete config classes instead of generic dictionary #1567

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Use concrete config classes instead of generic dictionary #1567

wants to merge 2 commits into from

Conversation

shaahji
Copy link
Contributor

@shaahji shaahji commented Jan 23, 2025

Use concrete config classes instead of generic dictionary

  • Pass now accepts a concrete BasePassConfig object instead of generic dict for config
  • Pass._run_for_config similarly accepts a concrete BasePassConfig object
  • Pass.generate_config returns a concrete BasePassConfig object
  • Removed "disable_search" argument from Pass.validate_config
  • Removed Pass.searialize_config. Use Pass.config.to_json or Pass.config.dict instead
  • Removed Pass._config_class, Use Pass.config.class instead
  • Updated all passes and tests to use input config as an object of type BasePassConfig instead of generic dict

Checklist before requesting a review

  • Add unit tests for this change.
  • Make sure all tests can pass.
  • Update documents if necessary.
  • Lint and apply fixes to your code by running lintrunner -a
  • Is this a user-facing change? If yes, give a description of this change to be included in the release notes.
  • Is this PR including examples changes? If yes, please remember to update example documentation in a follow-up PR.

(Optional) Issue link

@shaahji
Implement pass search
d63c35a 
Reimplement search logic to include passes in search space.
@shaahji
Use concrete config classes instead of generic dictionary
f360c64 
* Pass now accepts a concrete BasePassConfig object instead of generic
  dict for config
* Pass._run_for_config similarly accepts a concrete BasePassConfig
  object
* Pass.generate_config returns a concrete BasePassConfig object
* Removed "disable_search" argument from Pass.validate_config
* Removed Pass.searialize_config. Use Pass.config.to_json or
  Pass.config.dict instead
* Removed Pass._config_class, Use Pass.config.__class__ instead
* Updated all passes and tests to use input config as an object of type
  BasePassConfig instead of generic dict
@review-notebook-app Review Notebook App
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

github-advanced-security[bot]
github-advanced-security bot found potential problems Jan 23, 2025
View reviewed changes
examples/bert/user_script.py
import copy

import numpy as np
import torch
import torchmetrics
import transformers
from datasets import load_dataset, load_metric
from datasets import load_dataset

Check failure

Code scanning / lintrunner

MYPY/import Error

Cannot find implementation or library stub for module named "datasets" To disable, use # type: ignore[import]
examples/bert/user_script.py
try:
from datasets import load_metric
except ImportError:
from evaluate import load as load_metric

Check failure

Code scanning / lintrunner

MYPY/import Error

Cannot find implementation or library stub for module named "evaluate" To disable, use # type: ignore[import]
test/unit_test/search/test_search_strategy.py
import re
from collections import OrderedDict

import pytest

Check failure

Code scanning / lintrunner

MYPY/import Error test

Cannot find implementation or library stub for module named "pytest" To disable, use # type: ignore[import]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@shaahji