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

Issues with "Settings" dialog in MC #51

Open
slivkins opened this issue Jul 15, 2016 · 7 comments
Open

Issues with "Settings" dialog in MC #51

slivkins opened this issue Jul 15, 2016 · 7 comments

Comments

@slivkins
Copy link
Contributor

Major issues:

  • Some values (IDs, Keys and addresses) do not fit into the corresponding field, and cannot be viewed or copied in full. Which I assume is a bug.
  • Many of the settings are [probably] only for very advanced users. So perhaps it would be useful to label them as such and put them below the fold.
    • Specifically, I am not sure why anyone would want to use the following: Settings Address, Online Trainer, ASA Join Server, EH interaction string, EH observation string.

Minor issues:

  • AppID
    • AppID defaults to ResourceGroupName, and cannot be changed. Is this what we want? Then we may want to want people what whatever they choose for ResourceGroupName is going to be their AppID going forward.
    • Also, the explanation reads as if one can change the AppID (which would also change ResourceGroupName), which is not the case, right?
    • Point out that “Application” = instance of DS. So if they use multiple instances of DS for the same website, it is multiple “applications” from our PoV.
  • Azure Subscription ID:
    • this looks confusing, given that Azure subcrtiptions have names in plain text. Why not list the subscription name as well?
    • Reword explanation as “ID of the Azure subscription used for this deployment”.
  • AzureResourceGroup:
    • No way to see the explanation without clicking and opening a tab (with the resource group).
  • Application Insights:
    • The explanation talks about initializing the telemetry client, but it is not clear how to do that. Add more explanation, or maybe a link to FAQ? (Can we add URLs in these explanations?)
  • Setting blob address:
    • Sounds as “address for setting blob”. Rename to “Blob address for settings”? Even better, “Address for settings”, so that people don’t need to know what is “blob”?
    • If people need to know what is blob, then this should be briefly explained in the explanation.
  • Context type:
    • Rename to “Actions with features?” (or smth like that), and have two options, yes/no
    • In explanation, ADFs and “regular” features sounds mysterious. Say “Do actions come with features? Such features are then included in the context” (or smth like that).
  • Training frequency:
    • Currently the only option is “high”, whereas the explanation says that batch training is also available. So this is a little confusing.
    • Batch training is not done automatically, right? Then perhaps we should mention it.
  • Join timeout, Initial eps exploration: please see the “deploy DS blade” for better names and/or explanations.
  • VW arguments:
    • In explanation, seems useful to add smth like “Do not change unless you know what you are doing”, and add a pointer to the Wiki (wherever we talk about VW options).
@lhoang29
Copy link
Contributor

  1. Some values (IDs, Keys and addresses) do not fit into the corresponding field, and cannot be viewed or copied in full. Which I assume is a bug.
    You should be able to copy the values even though the box is short, I can increase it.
  2. Many of the settings are [probably] only for very advanced users. So perhaps it would be useful to label them as such and put them below the fold.
    Specifically, I am not sure why anyone would want to use the following: Settings Address, Online Trainer, ASA Join Server, EH interaction string, EH observation string.

    These are generally useful for programmatic access (e.g. resetting the online trainer, upload data to EH etc...).
  3. AppID defaults to ResourceGroupName, and cannot be changed. Is this what we want? Then we may want to want people what whatever they choose for ResourceGroupName is going to be their AppID going forward.
    Also, the explanation reads as if one can change the AppID (which would also change ResourceGroupName), which is not the case, right?

    We don't support changing the resource group name, maybe better to not call it App ID and just keep using Resource Group name to be consistent with the deployment process. I'll update the explanation.
  4. Batch training is not done automatically, right? Then perhaps we should mention it.
    Right, I'll update the comment. At one point I wanted to have a different training service for low frequency model update that could be spun up/down on demand to save cost. However, the on demand part was not natively supported in Azure (yet) and cost may not be that big of an issue so we ended up excluding it from the release for simplicity.

Other comments sound good.

@slivkins
Copy link
Contributor Author

@settings for advanced users: "useful for programmatic access" suggests "only for advanced users", eh? Two suggestions: keep such things under the fold, and add explanation for why/when you might want to need them.

@APPID and Resource Group Name: So you mean to remove AppID from settings altogether, and use Resource Group Name instead? Makes sense to me.

@Training speed: actually, if now there is no option, then why have this option at all?

Also:
@values that don't fit: can you make the fields scrollable? Not a big deal if there is a way to copy in full (which I didn't find, but dumb me).

@slivkins slivkins reopened this Jul 18, 2016
@lhoang29
Copy link
Contributor

Most of your comments are already addressed and checked in, can you have a look? The only thing I didn't do was the folding of advanced usage mode, it's not clear to me how we'd classify which option is or is not advanced. The page is simple enough that we don't need to organize it further IMHO. I also sort of like the fact that these options are obvious/visible.

@slivkins
Copy link
Contributor Author

slivkins commented Jul 18, 2016

Thanks! Sorry, couldn't take a look yet, as as internet is a bit flaky on my train ... Was planning to take a look later in the afternoon. In the meantime, I was just responding to your comments.

@advanced options, I (still) think having less options helps, but I agree such change would require some [more] consensus. I'll bug @JohnLangford and/or @danmelamed.

@slivkins
Copy link
Contributor Author

@lhoang29 Took another look at the MC. Remaining issues:

@Azure Resource Group: No way to see the explanation without clicking and opening a tab (with the resource group).

@"advanced options" - will follow up via email in a sec.

@lhoang29
Copy link
Contributor

lhoang29 commented Aug 1, 2016

@slivkins, did we ever reach agreement on this?

@slivkins
Copy link
Contributor Author

slivkins commented Aug 2, 2016

@lhoang29 Well, my previous msg still stands. In Azure Resource Group, there is no way to see the explanation without clicking and opening a new tab. And splitting off the advanced options made sense to John, in general, I've sent out an email a while back for the specifics (copied below), and noone responded.


Hi. So, several options in MC seem like they are only useful for advanced users who know what they are doing. I suggest to call them “advanced options” and bring them under the fold. Specifically, the following things seem “advanced-only”:

Azure subscription ID
Application insights
Address for settings
[Address for] online trainer
ASA policy Join Server
VW arguments
Azure storage connection string
EH interaction connection string
EH observation connection string

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

No branches or pull requests

2 participants