Skip to content

add simulation ui params#63

Merged
chenweize1998 merged 2 commits intoOpenBMB:mainfrom
JetSquirrel:add_ui_params
Oct 15, 2023
Merged

add simulation ui params#63
chenweize1998 merged 2 commits intoOpenBMB:mainfrom
JetSquirrel:add_ui_params

Conversation

@JetSquirrel
Copy link
Copy Markdown
Contributor

issue #61

Now we add parameters in simulation_ui:

  • server_name: Default server name is "127.0.0", now you can change server name, such as "0.0.0.0".
  • share: If want create a publicly shareable link, you can set value as true, yes, y or 1

for example

python3 agentverse_command/main_simulation_gui.py --task simulation/sde_team/sde_team_3players --share true --server_name 0.0.0.0

@chenweize1998
Copy link
Copy Markdown
Collaborator

Thanks! Actually I think you don't need to define the is_share function to parse the boolean value for the share variable. You can simply use action="store_true", so that when the user specifies --share, the args.share will be True, otherwise it will be False. Do you think it will be better? Or do you have any other considerations that make is_share function necessary?

@JetSquirrel
Copy link
Copy Markdown
Contributor Author

Thanks for your review!
User must set parameter as --share True when direct use type=bool, I think parameters is lower case letters, --share True is unexpected.
So I use is_share to bring more convenience for set parameter, and we can use --share true, --share yes, --share y .
But it's look like a bad choice.

Do you think this method is better?

parser.add_argument("--share",
                        type=str,
                        default="false",
                        help="Create a publicly shareable link")

ui_kwargs={"share":True if args.share in ("true","True","yes","y") else False,
                                                  "server_name":args.server_name}

@chenweize1998
Copy link
Copy Markdown
Collaborator

I feel that a common practice when using argparse is to use action="store_true" when specifying boolean variables of this kind. Additionally, I personally prefer the original method over the recent approach mentioned in the replies. All these are different implementation to the same functionality, but I would lean towards directly using action to specify share. Can you try to use action?

@JetSquirrel
Copy link
Copy Markdown
Contributor Author

Thanks your advice, I got it.
I updated code in commit c63bc28, please review it again.

@chenweize1998
Copy link
Copy Markdown
Collaborator

Thanks! Merging this now.

@chenweize1998 chenweize1998 merged commit 5292621 into OpenBMB:main Oct 15, 2023
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.

2 participants