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

feat: add model-based algorithms #212

Merged
merged 37 commits into from
May 2, 2023

Conversation

hdadong
Copy link
Collaborator

@hdadong hdadong commented Apr 11, 2023

feat: add model-based algorithms

Types of changes

What types of changes does your code introduce? Put an x in all the boxes that apply:

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds core functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation (update in the documentation)

Checklist

Go over all the following points, and put an x in all the boxes that apply.
If you are unsure about any of these, don't hesitate to ask. We are here to help!

  • I have read the CONTRIBUTION guide. (required)
  • My change requires a change to the documentation.
  • I have updated the tests accordingly. (required for a bug fix or a new feature)
  • I have updated the documentation accordingly.
  • I have reformatted the code using make format. (required)
  • I have checked the code using make lint. (required)
  • I have ensured make test pass. (required)

omnisafe/adapter/modelbased_adapter.py Outdated Show resolved Hide resolved
omnisafe/adapter/modelbased_adapter.py Show resolved Hide resolved
omnisafe/adapter/modelbased_adapter.py Outdated Show resolved Hide resolved
omnisafe/adapter/modelbased_adapter.py Outdated Show resolved Hide resolved
omnisafe/adapter/modelbased_adapter.py Outdated Show resolved Hide resolved
omnisafe/algorithms/model_based/planner/arc.py Outdated Show resolved Hide resolved
omnisafe/algorithms/model_based/planner/arc.py Outdated Show resolved Hide resolved
omnisafe/algorithms/model_based/planner/arc.py Outdated Show resolved Hide resolved
omnisafe/algorithms/model_based/planner/arc.py Outdated Show resolved Hide resolved
omnisafe/algorithms/model_based/planner/cap.py Outdated Show resolved Hide resolved
omnisafe/algorithms/model_based/cap_pets.py Outdated Show resolved Hide resolved
omnisafe/algorithms/model_based/cce_pets.py Outdated Show resolved Hide resolved
omnisafe/algorithms/model_based/rce_pets.py Outdated Show resolved Hide resolved
omnisafe/algorithms/model_based/safeloop.py Outdated Show resolved Hide resolved
omnisafe/algorithms/model_based/base/pets.py Outdated Show resolved Hide resolved
@hdadong hdadong force-pushed the dev-modelbase-plan branch 2 times, most recently from c37c51f to 602f6a2 Compare April 17, 2023 08:13
@codecov-commenter
Copy link

codecov-commenter commented Apr 17, 2023

Codecov Report

Merging #212 (e7a004f) into dev (c1c6993) will decrease coverage by 1.01%.
The diff coverage is 93.46%.

❗ Current head e7a004f differs from pull request most recent head 5f92d0a. Consider uploading reports for the commit 5f92d0a to get more accurate results

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##              dev     #212      +/-   ##
==========================================
- Coverage   97.00%   95.99%   -1.01%     
==========================================
  Files          99      118      +19     
  Lines        4331     5987    +1656     
==========================================
+ Hits         4201     5747    +1546     
- Misses        130      240     +110     
Impacted Files Coverage Δ
omnisafe/algorithms/algo_wrapper.py 94.68% <33.33%> (-2.02%) ⬇️
omnisafe/envs/safety_gymnasium_modelbased.py 80.75% <80.75%> (ø)
omnisafe/evaluator.py 92.20% <88.89%> (-2.09%) ⬇️
omnisafe/algorithms/model_based/base/ensemble.py 93.02% <93.02%> (ø)
omnisafe/algorithms/model_based/base/loop.py 93.10% <93.10%> (ø)
omnisafe/algorithms/model_based/planner/rce.py 93.33% <93.33%> (ø)
omnisafe/envs/mujoco_env.py 93.33% <93.33%> (ø)
omnisafe/adapter/modelbased_adapter.py 93.39% <93.39%> (ø)
...mnisafe/algorithms/model_based/planner/safe_arc.py 96.05% <96.05%> (ø)
omnisafe/algorithms/model_based/safeloop.py 96.67% <96.67%> (ø)
... and 16 more

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@hdadong hdadong force-pushed the dev-modelbase-plan branch 5 times, most recently from 2777eda to 5454ed9 Compare April 19, 2023 07:41
@hdadong hdadong force-pushed the dev-modelbase-plan branch 3 times, most recently from a2874f9 to 0346df5 Compare April 27, 2023 17:02
Copy link
Member

@zmsn-2077 zmsn-2077 left a comment

Choose a reason for hiding this comment

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

LGTM.

example:

```python
path ='/home/username/omnisafe/omnisafe/examples/benchmarks/exp-x/Model-Based-Benchmarks'
Copy link
Member

Choose a reason for hiding this comment

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

May be relative path better. e.g. ./examples/benchmarks/exp-x/Model-Based-Benchmarks

kwargs(dict): The other keyword arguments.

Attributes:
_env_id (str): The environment id.
Copy link
Member

Choose a reason for hiding this comment

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

private attributes may not need docs.

self._last_dynamics_update = 0
self._last_policy_update = 0
self._last_eval = 0
self._first_log = False
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
self._first_log = False
self._first_log: bool = False

The same with others.

Copy link
Member

@Gaiejj Gaiejj left a comment

Choose a reason for hiding this comment

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

Some problems still exist, but it doesn't matter, I will open another PR to fix my issue and I will request your review.

"""Get lidar from numpy coordinate.

Args:
obs (np.ndarray): The observation.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
obs (np.ndarray): The observation.
obs (torch.Tensor): The observation.

else None
)

def render(self, *args: str, **kwargs: int) -> Any:
Copy link
Member

Choose a reason for hiding this comment

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

why the **kwargs is int type ?

},
)

def _reset_log(self, idx: int | None = None) -> None: # pylint: disable=unused-argument
Copy link
Member

Choose a reason for hiding this comment

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

why unused-argument idx maintained ?

@zmsn-2077
Copy link
Member

Open a fix request, then fix this code details.

@zmsn-2077 zmsn-2077 merged commit d65f37f into PKU-Alignment:dev May 2, 2023
4 checks passed
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

5 participants