-
Notifications
You must be signed in to change notification settings - Fork 25
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
Safety valve to seperate the DSE execution with benchmarking execution #334
base: main
Are you sure you want to change the base?
Changes from 17 commits
d945201
1be4398
4df4ab9
e6905f7
177694f
b10dbfb
15be693
d5d1e14
96ab055
8cab450
0acf43e
55c203a
be56e31
71a12c4
669bd8d
b055d4d
02bb3e0
382e424
e16873d
fd3b6c9
9fadfce
4cb3f02
996d6fd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,102 @@ | ||
# SPDX-FileCopyrightText: NVIDIA CORPORATION & AFFILIATES | ||
# Copyright (c) 2024 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
# SPDX-License-Identifier: Apache-2.0 | ||
# | ||
# Licensed under the Apache License, Version 2.0 (the "License"); | ||
# you may not use this file except in compliance with the License. | ||
# You may obtain a copy of the License at | ||
# | ||
# http://www.apache.org/licenses/LICENSE-2.0 | ||
# | ||
# Unless required by applicable law or agreed to in writing, software | ||
# distributed under the License is distributed on an "AS IS" BASIS, | ||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
# See the License for the specific language governing permissions and | ||
# limitations under the License. | ||
|
||
from abc import ABC, abstractmethod | ||
from typing import Any, Dict, Optional, Tuple | ||
|
||
|
||
class BaseGym(ABC): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we merge BaseGym and CloudAIGym into single object? Why do we need both? |
||
"""Base class for CloudAI Gym environments.""" | ||
|
||
def __init__(self): | ||
"""Initialize the CloudAIGym environment.""" | ||
self.action_space = self.define_action_space() | ||
self.observation_space = self.define_observation_space() | ||
|
||
@abstractmethod | ||
def define_action_space(self) -> Dict[str, Any]: | ||
""" | ||
Define the action space for the environment. | ||
|
||
Returns: | ||
Dict[str, Any]: The action space. | ||
""" | ||
pass | ||
|
||
@abstractmethod | ||
def define_observation_space(self) -> list: | ||
""" | ||
Define the observation space for the environment. | ||
|
||
Returns: | ||
list: The observation space. | ||
""" | ||
pass | ||
|
||
@abstractmethod | ||
def reset( | ||
self, seed: Optional[int] = None, _options: Optional[dict[str, Any]] = None | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please rename |
||
) -> Tuple[list, dict[str, Any]]: | ||
""" | ||
Reset the environment. | ||
|
||
Args: | ||
seed (Optional[int]): Seed for the environment's random number generator. | ||
options (Optional[dict]): Additional options for reset. | ||
|
||
Returns: | ||
Tuple: A tuple containing: | ||
- observation (list): Initial observation. | ||
- info (dict): Additional info for debugging. | ||
""" | ||
pass | ||
|
||
@abstractmethod | ||
def step(self, action: Any) -> Tuple[list, float, bool, dict]: | ||
""" | ||
Execute one step in the environment. | ||
|
||
Args: | ||
action (Any): Action chosen by the agent. | ||
|
||
Returns: | ||
Tuple: A tuple containing: | ||
- observation (list): Updated system state. | ||
- reward (float): Reward for the action taken. | ||
- done (bool): Whether the episode is done. | ||
- info (dict): Additional info for debugging. | ||
""" | ||
pass | ||
|
||
@abstractmethod | ||
def render(self, mode: str = "human"): | ||
""" | ||
Render the current state of the environment. | ||
|
||
Args: | ||
mode (str): The mode to render with. Default is "human". | ||
""" | ||
pass | ||
|
||
@abstractmethod | ||
def seed(self, seed: Optional[int] = None): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
""" | ||
Set the seed for the environment's random number generator. | ||
|
||
Args: | ||
seed (Optional[int]): Seed for the environment's random number generator. | ||
""" | ||
pass |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -53,6 +53,7 @@ class TestRun: | |
output_path: Path = Path("") | ||
iterations: int = 1 | ||
current_iteration: int = 0 | ||
dse_iteration: int = 0 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why can't we use existing |
||
time_limit: Optional[str] = None | ||
sol: Optional[float] = None | ||
weight: float = 0.0 | ||
|
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 do not agree with the proposed directory structure. We need to discuss this with all core members: Srinivas, Srivatsan, and Andrei.
My proposal
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 would suggest that we keep the structure the same for both regular and dse cases: