Modify legal actions of rocksample #84
Conversation
zkytony
left a comment
zkytony
left a comment
There was a problem hiding this comment.
Thank you! Great work.
Requesting a few changes; see comments.
Also, could you add some unit tests? Those can go in pomdp_py/tests. There's a custom test framework here and it should be easy to understand. These tests are run by CI so ensures your work is preserved in a working state.
| print(f"Max total reward: {max(total_rewards)}") | ||
| print(f"Min discounted reward: {min(total_discounted_rewards):.3f}") | ||
| print(f"Max discounted reward: {max(total_discounted_rewards):.3f}") | ||
| print("="*50) |
There was a problem hiding this comment.
Could you separate this part into a function called run_experiment or benchmark? Then, if a user runs
python -m pomdp_py -r rocksample --benchmark
it will run this. Otherwise, it still runs a single-run example?
|
@HarryXuancy also notice the pre-commit check fails. |
|
@HarryXuancy I see this has become stale. I'll see if I can get this pass CI. Also I noticed two of my comments were resolved without reason. Specifically, it doesn't look like the code is generating output.txt files so I don't think we need to add it in gitignore. |
|
@HarryXuancy please check out this one #85 |
Sorry for my big delay! And I resolved the comments because of my personal habit. If I think a comment is absolutely correct, I usually just apply the corresponding change without adding a reply. Sorry I probably should have added some responses before resolving the comments. |
* Modify legal actions of rocksample * allow --benchmark in rocksample * pre-commit * remove gitignore * minor hash improvement --------- Co-authored-by: Chunyu Xuan <lhsxxcy@126.com>
|
Merged by #85 |
2 participants
In this PR I modify the legal actions in RockSample environment:
These new settings align with Silver's cpp code and can bring better results since the search space is smaller. I am not sure if pomdp-py wants to adopt this new setting, you can decide whether to merge based on the need.