-
-
Notifications
You must be signed in to change notification settings - Fork 39
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
[REVIEW]: Abmarl: Connecting Agent-Based Simulations with Multi-Agent Reinforcement Learning #3424
Comments
Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @seba-1511, @abhiramm7 it looks like you're currently assigned to review this paper 🎉. Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post. ⭐ Important ⭐ If you haven't already, you should seriously consider unsubscribing from GitHub notifications for this (https://github.com/openjournals/joss-reviews) repository. As a reviewer, you're probably currently watching this repository which means for GitHub's default behaviour you will receive notifications (emails) for all reviews 😿 To fix this do the following two things:
For a list of things I can do to help you, just type:
For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:
|
|
|
@rusu24edward if you could take a look at the above missing DOI please. |
Reviewers, please note that the release branch is abmarl-87-interface-release. The main branch has some additional developmental work that is not a part of the release nor this review. |
@whedon commands |
Here are some things you can ask me to do:
|
@whedon check references |
|
@drvinceknight I believe I have fixed the DOI issue. |
👋 @seba-1511, please update us on how your review is going (this is an automated reminder). |
👋 @abhiramm7, please update us on how your review is going (this is an automated reminder). |
I plan to complete my review by Sunday, July 18. |
I plan to wrap up the review by early next week, July 19 |
Hi @seba-1511 and @abhiramm7. Thanks for the review work you have done so far. I just wanted to check in since we are at the dates now where we've anticipated the review would be done. Is there anything you need from me to help move this along? Any feedback on the software or paper? |
@rusu24edward Apologies for the delay -- so far the submission looks good. I think ray in requirements.txt should be updated to Traceback (most recent call last):
File "/Users/sebarnol/anaconda3/envs/abmarl36/lib/python3.6/site-packages/redis/connection.py", line 559, in connect
sock = self._connect()
File "/Users/sebarnol/anaconda3/envs/abmarl36/lib/python3.6/site-packages/redis/connection.py", line 615, in _connect
raise err
File "/Users/sebarnol/anaconda3/envs/abmarl36/lib/python3.6/site-packages/redis/connection.py", line 603, in _connect
sock.connect(socket_address)
TimeoutError: [Errno 60] Operation timed out
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "/Users/sebarnol/anaconda3/envs/abmarl36/bin/abmarl", line 33, in <module>
sys.exit(load_entry_point('abmarl', 'console_scripts', 'abmarl')())
File "/Users/sebarnol/Desktop/Abmarl/abmarl/scripts/scripts.py", line 43, in cli
train.run(path_config)
File "/Users/sebarnol/Desktop/Abmarl/abmarl/scripts/train_script.py", line 17, in run
train.run(full_config_path)
File "/Users/sebarnol/Desktop/Abmarl/abmarl/train.py", line 29, in run
ray.init()
File "/Users/sebarnol/anaconda3/envs/abmarl36/lib/python3.6/site-packages/ray/_private/client_mode_hook.py", line 62, in wrapper
return func(*args, **kwargs)
File "/Users/sebarnol/anaconda3/envs/abmarl36/lib/python3.6/site-packages/ray/worker.py", line 797, in init
ray_params=ray_params)
File "/Users/sebarnol/anaconda3/envs/abmarl36/lib/python3.6/site-packages/ray/node.py", line 230, in __init__
self.start_head_processes()
File "/Users/sebarnol/anaconda3/envs/abmarl36/lib/python3.6/site-packages/ray/node.py", line 861, in start_head_processes
self.start_redis()
File "/Users/sebarnol/anaconda3/envs/abmarl36/lib/python3.6/site-packages/ray/node.py", line 686, in start_redis
port_denylist=self._ray_params.reserved_ports)
File "/Users/sebarnol/anaconda3/envs/abmarl36/lib/python3.6/site-packages/ray/_private/services.py", line 891, in start_redis
primary_redis_client.set("NumRedisShards", str(num_redis_shards))
File "/Users/sebarnol/anaconda3/envs/abmarl36/lib/python3.6/site-packages/redis/client.py", line 1801, in set
return self.execute_command('SET', *pieces)
File "/Users/sebarnol/anaconda3/envs/abmarl36/lib/python3.6/site-packages/redis/client.py", line 898, in execute_command
conn = self.connection or pool.get_connection(command_name, **options)
File "/Users/sebarnol/anaconda3/envs/abmarl36/lib/python3.6/site-packages/redis/connection.py", line 1192, in get_connection
connection.connect()
File "/Users/sebarnol/anaconda3/envs/abmarl36/lib/python3.6/site-packages/redis/connection.py", line 563, in connect
raise ConnectionError(self._error_message(e))
redis.exceptions.ConnectionError: Error 60 connecting to 10.142.140.7:6379. Operation timed out.
[1] 87008 terminated abmarl train examples/multi_corridor_example.py Specifically, I ran the following: conda create -n abmarl36 python=3.6
conda activate abmarl36
git clone https://github.com/LLNL/Abmarl/
cd Abmarl
pip install -r requirements.txt
pip install -e . --no-deps
abmarl train examples/multi_corridor_example.py |
Other than that (and once I can confirm functionality), my only minor comment is on state of need/scholarship. The discussion around existing software around multi-agent RL could be more thorough; for example, here's some popular software that could be discussed:
|
@rusu24edward I've created two issues 168 and 167 regarding installation and documentation. The examples and documentation were really thorough; I would just recommend adding a note on dependencies in the installation instructions and also providing links the python files (e.g., |
Thanks for the reviews! I will try to address each of these points by the end of the week. |
@seba-1511 Regarding your comment on other popular software that can be addressed, I'd be happy to include more discussion on comparisons with popular software, but I think it's important to clarify the suggestions you made as it will shed light on what ABMARL targets. Reinforcement learning has two components: simulation and algorithm (also commonly referred to as environment and agent). Our software targets the simulation component, so I restricted my discussion to other software that specifically targets the simulation component. Regarding your suggestions: MADDPG pymarl MAAC SMARTS and MARLO |
@seba-1511 Thanks for bringing this to my attention. I just ran a fresh virtual environment and did not get any error. We technically specify that python 3.7+ is required, can you try this again with python3.7? |
@abhiramm7 I've added links to the tutorials as you suggested. |
|
|
👋 @openjournals/joss-eics, this paper is ready to be accepted and published. Check final proof 👉 openjournals/joss-papers#2523 If the paper PDF and Crossref deposit XML look good in openjournals/joss-papers#2523, then you can now move forward with accepting the submission by compiling again with the flag
|
@openjournals/joss-eics note that the branch from which the paper is to be built is |
@drvinceknight - I'm confused - you didn't do |
It appears the paper is the same in the branch and main, so I don't think it matters, but perhaps @drvinceknight or @rusu24edward could explain before we proceed? In the meantime, I'll proofread the paper (and if changes are needed, suggest them in main) |
👋 @rusu24edward - in any case (ignoring the branch vs main issue), there are a couple of small changes needed that I've indicated in LLNL/Abmarl#195 If the paper in main is ok, then these can just be merged and we can proceed to publication with that branch of the paper. If the paper in the other branch is different, these changes can be made there and we can publish using the paper in that branch. But again, I really don't understand what's going on with these two branches, as the paper appears the same in both, and that's the only thing that JOSS uses the branch for. |
Apologies, I didn't realise that's what I should have done. Thanks for pointing it out. |
@danielskatz I'm sorry for the confusion, I'll do my best to explain. Our software utilizes a release strategy similar to the branch release stragey. We have our main branch, which contains active development that is not yet ready for release, and when we make releases we create a release branch off main. This review has been of our first release, which is on abmarl-87-interface-release, not the main branch. The paper is the same on both branches. Submitting the changes to main is fine, I have duplicated them for the release branch. |
The JOSS publication is of the paper, and it includes a pointer to the full repository, so there's no need to worry about the branch in this case, I don't think. Does this make sense? |
@danielskatz Yes, that makes sense. |
@whedon recommend-accept |
|
|
👋 @openjournals/joss-eics, this paper is ready to be accepted and published. Check final proof 👉 openjournals/joss-papers#2525 If the paper PDF and Crossref deposit XML look good in openjournals/joss-papers#2525, then you can now move forward with accepting the submission by compiling again with the flag
|
@whedon accept deposit=true |
|
🐦🐦🐦 👉 Tweet for this paper 👈 🐦🐦🐦 |
🚨🚨🚨 THIS IS NOT A DRILL, YOU HAVE JUST ACCEPTED A PAPER INTO JOSS! 🚨🚨🚨 Here's what you must now do:
Any issues? Notify your editorial technical team... |
Congratulations to @rusu24edward (Edward Rusu) and co-author!! And thanks to @drvinceknight for editing, and @seba-1511 and @abhiramm7 for reviewing! |
🎉🎉🎉 Congratulations on your paper acceptance! 🎉🎉🎉 If you would like to include a link to your paper from your README use the following code snippets:
This is how it will look in your documentation: We need your help! Journal of Open Source Software is a community-run journal and relies upon volunteer effort. If you'd like to support us please consider doing either one (or both) of the the following:
|
Thank you everyone very much for this awesome review process and publication. Looking forward to more! |
Submitting author: @rusu24edward (Edward Rusu)
Repository: https://github.com/LLNL/Abmarl/
Version: 0.1.4
Editor: @drvinceknight
Reviewer: @seba-1511, @abhiramm7
Archive: 10.5281/zenodo.5196791
Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post.
Status
Status badge code:
Reviewers and authors:
Please avoid lengthy details of difficulties in the review thread. Instead, please create a new issue in the target repository and link to those issues (especially acceptance-blockers) by leaving comments in the review thread below. (For completists: if the target issue tracker is also on GitHub, linking the review thread in the issue or vice versa will create corresponding breadcrumb trails in the link target.)
Reviewer instructions & questions
@seba-1511 & @abhiramm7, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:
The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @drvinceknight know.
✨ Please start on your review when you are able, and be sure to complete your review in the next six weeks, at the very latest ✨
Review checklist for @seba-1511
Conflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
Review checklist for @abhiramm7
Conflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
The text was updated successfully, but these errors were encountered: