-
Notifications
You must be signed in to change notification settings - Fork 26
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
chore: remove CPE benchmark and cleanup related code #571
chore: remove CPE benchmark and cleanup related code #571
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you. The PR looks good to me!
# set validate item | ||
item = dict() | ||
item["pod"] = benchmark_filename | ||
item["scenarioID"] = "" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we may consider remove scenarioID column or replace later once we have to a tool or a way to differentiate the scenario of the benchmark.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure will create an issue to track this
7cd9b02
to
dd89d0b
Compare
src/kepler_model/cmd/main.py
Outdated
# Plot arguments | ||
parser.add_argument("--target-data", type=str, help="Speficy target plot data (preprocess, estimate)") | ||
parser.add_argument("--scenario", type=str, help="Speficy scenario") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sunya-ch Forgot to remove these. Will again require your lgtm
@@ -1035,10 +945,6 @@ def run(): | |||
parser.add_argument("-fg", "--feature-group", type=str, help="Specify target feature group for energy estimation.", default="") | |||
parser.add_argument("--model-name", type=str, help="Specify target model name for energy estimation.") | |||
|
|||
# Plot arguments | |||
parser.add_argument("--target-data", type=str, help="Speficy target plot data (preprocess, estimate)") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to keep #Plot arguments and --target-data (also would be nice to fix typo of Specify). It is used by plot command.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my bad I thought it was related to plot_scenario
func 😞
This commit removes the deployments and benchmark related to the CPE operator. It also cleans up the associated code and documentation. Signed-off-by: vprashar2929 <[email protected]>
dd89d0b
to
b210c67
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thank you.
This commit removes the deployments and benchmark related to the CPE operator. It also cleans up the associated code and documentation.
Addresses #560
Checklist for PR Author
In addition to approval, the author must confirm the following check list:
Run the following command to format your code:
Create issues for unresolved comments and link them to this PR. Use one of the following labels:
must-fix
: The logic appears incorrect and must be addressed.minor
: Typos, minor issues, or potential refactoring for better readability.nit
: Trivial issues like extra spaces, commas, etc.