-
Notifications
You must be signed in to change notification settings - Fork 7
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
MCS-1923 MCS-1924 Webenabled Updates: Previous Images, Quick Actions #706
MCS-1923 MCS-1924 Webenabled Updates: Previous Images, Quick Actions #706
Conversation
ThomasSchellenbergNextCentury
commented
Oct 25, 2023
•
edited
Loading
edited
- Now shows a sideways-scrolling list of small images from the latest 500 actions (see comment)
- Added "quick action" buttons to run multiple actions in a row and watch results as they become available
- Renamed "Keyboard Shortcuts" to "Usage" and added a "Notes" section
- Minor styling tweaks and bug fixes
…uick action buttons.
function runActionsHelper(actionList, autofill) { | ||
const response = fetch("{{url_for('handle_keypress')}}", | ||
{ | ||
method: 'POST', | ||
headers: { | ||
'Content-Type': 'application/json' | ||
}, | ||
body: JSON.stringify({ | ||
"action": actionList[0], | ||
"objectImageCoordsX": params.objectImageCoordsX, | ||
"objectImageCoordsY": params.objectImageCoordsY, | ||
"force": document.getElementById("force").value, | ||
"clockwise": document.getElementById("clockwise").checked, | ||
"lateral": document.getElementById("lateral").value, | ||
"straight": document.getElementById("straight").value | ||
}) | ||
}) | ||
.then(parseJsonResponse) | ||
.then(updateStepNumber) | ||
.then(updateActionList) | ||
.then(updateOutputInfo) | ||
.then(updateImages) | ||
.then(function(data) { | ||
if (autofill) { | ||
var requiredActionList = data.action_list.split(", "); | ||
if (requiredActionList.length == 1) { | ||
actionList.push(requiredActionList[0]); | ||
} | ||
} | ||
if (actionList.length == 1) { | ||
processingKeypress = false | ||
loadingSpinner(false) | ||
} else { | ||
runActionsHelper(actionList.slice(1), autofill); | ||
} | ||
}); | ||
} | ||
|
||
function runMultipleActions(action, totalCount) { | ||
console.log('run multiple actions: ' + action + ' x ' + totalCount); | ||
processingKeypress = true; | ||
loadingSpinner(true); | ||
|
||
var actionList = []; | ||
for (let i = 0; i < totalCount; i++) { | ||
actionList.push(action); | ||
} | ||
runActionsHelper(actionList, false); | ||
} | ||
|
||
function runRequiredActions(startingAction) { | ||
console.log('run required actions'); | ||
processingKeypress = true; | ||
loadingSpinner(true); | ||
|
||
runActionsHelper([startingAction], true); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the "quick actions", I decided to perform the actions one at a time, updating the webpage to display their images while the spinner is running. This way, the user can watch as stuff is happening.
imagesHtml = ''; | ||
previousStep = data.step - 1; | ||
for (let previousImage of data.previous_images) { | ||
imagesHtml = imagesHtml + ( | ||
'<div class="column">' + | ||
'<div>Step ' + previousStep + '</div>' + | ||
'<img width="300" height="200" src="' + previousImage + | ||
'" alt="step ' + previousStep + '"/>' + | ||
'</div>' | ||
); | ||
previousStep--; | ||
} | ||
var previousImages = document.getElementById('previousImages'); | ||
previousImages.innerHTML = imagesHtml; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't sure if there's a better way than this of adding elements dynamically since we're not using a framework like React.
var actionButton = document.getElementById('runRequiredActions'); | ||
var requiredActionList = data.action_list.split(", "); | ||
if (requiredActionList.length == 1) { | ||
actionButton.onclick = function(event) { | ||
runRequiredActions(requiredActionList[0]); | ||
} | ||
actionButton.removeAttribute('disabled'); | ||
} else { | ||
actionButton.onclick = function(event) { /* no-op */ } | ||
actionButton.setAttribute('disabled', ''); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This updates the "quick action" button that will run all of the "required actions" for you.
webenabled/templates/mcs_page.html
Outdated
<p><strong>Keyboard Shortcuts</strong></p> | ||
<p>Below are the keyboard shortcuts. Make sure your mouse is somewhere in the | ||
browser window and key strokes will be sent to the environment.</p> | ||
|
||
<img id="keyboard" width="400" src="static/keyboard.jpg" alt="keyboard" class="center"/> | ||
<p><strong>Usage</strong></p> | ||
<p>Press a key to run the corresponding action:</p> | ||
|
||
<p>The left part of the keyboard is for <strong>motion</strong> | ||
so that 'a' and 'd' make the agent move left or right without rotating. | ||
To <strong>rotate</strong> left and right, use 'j' and 'l' (lower case 'L') </p> | ||
<img id="keyboard" width="480" src="static/keyboard.jpg" alt="keyboard" class="center"/> | ||
|
||
<p>Notes:</p> | ||
<ul id="keyboardInfo"> | ||
<li>If an action is not successful, you will see the reason in the "Return Status" output.</li> | ||
<li>The W, A, S, and D keys let you move 0.1 meter in a specific direction, but do not change your facing.</li> | ||
<li>The I, J, K, and L keys let you look up/down or rotate left/right 10 degrees, but do not make you move.</li> | ||
<li>The 1-9 keys and the M key let you interact with objects in the world. Before pressing a key, click on the object in the current image to update the image coordinate parameters used for the action (the default is the center of the image). | ||
<ul> | ||
<li>The PullObject action applies physical force at the center of the object going directly toward you. Set the "force" parameter between 0 and 1.</li> | ||
<li>The PushObject action applies physical force at the center of the object going directly away from you. Set the "force" parameter between 0 and 1.</li> | ||
<li>The TorqueObject action applies physical force at the center of the object going clockwise (if positive) or counter-clockwise (if negative). Set the "force" parameter between -1 and 1.</li> | ||
<li>The MoveObject action moves the object a distance of 0.1 meter in up to two directions based on your current facing. Set the "straight" parameter to either -1, 1, or 0 to move the object toward you (-1), away from you (1), or neither (0). Set the "lateral" parameter to either -1, 1, or 0 to move the object left (-1), right (1), or neither direction (0).</li> | ||
<li>The RotateObject action rotates the object exactly 5 degrees. Uncheck the "clockwise" parameter to adjust the direction.</li> | ||
</ul> | ||
</li> | ||
<li>The T key lets you "interact" with another agent in the environment. Some tasks require you to interact with an agent before it will produce the soccer ball.</li> | ||
<li>The H key lets you end the current stage of this trial. Only use when required. You will see a black image on this step. Sometimes you are teleported to another area of the room during this step.</li> | ||
<li>The Spacebar lets you pass for one step, taking no action but advancing the physics simulation.</li> | ||
<li>Some scenes start with "required actions", meaning that for one or more steps there is only one available action you can take. Trying to use a different action will cause an error.</li> | ||
<li>Using Shift or Caps Lock is not necessary.</li> | ||
<li>Having trouble? Click on the current image in your web browser to restore focus, and keep your cursor within the browser window.</li> | ||
</ul> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A bit out of scope for this ticket, but I expanded the Usage section to include better descriptions of the different actions.
.section { | ||
margin: 0.75em; | ||
margin-left: 20px; | ||
margin-top: 20px; | ||
background-color: #FFFFFF; | ||
padding-left: 5px; | ||
padding-right: 5px; | ||
padding: 10px; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The section margins seemed a bit irregular -- I believe I've addressed them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think my nitpick was with the "Scenes" header on the left not lining up right with the "Scene Name" in the middle, which is back now, but if it only bothers me I can let it go lol.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, now that you've pointed it out, it's bothering me too! I just pushed a fix -- let me know what you think.
margin-right: 4px; | ||
margin-right: 24px; | ||
width: 100px; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made the text input fields smaller, since they seemed unnecessarily big.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a heads up, they're a bit wider now for me (perhaps there's an OS difference here?) but its not obnoxious or anything.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, yeah, must depend on the OS or browser.
<div>Amount: | ||
<input type="number" min="0" max="1" step="0.1" value="1" id="amount"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed "Amount", since it's only actually used for Open and Close, and we don't have any reason to open or close containers halfway.
<div> | ||
Parameter(s) will be used if applicable for chosen keyboard shortcut, | ||
see <a href="https://nextcenturycorporation.github.io/MCS/api.html#machine_common_sense.Action">Action API</a> | ||
for more information on parameters. If any of the inputs are not valid, an error message will be displayed, | ||
and your chosen action will not be processed until error(s) are resolved. | ||
</div> | ||
<div> | ||
For image coordinates, the default is the center of the scene image. | ||
Click a point on the Unity image to populate with a different coordinate. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed this since similar info is now in the Notes under Usage.
IMAGE_WAIT_TIMEOUT = 20.0 | ||
IMAGE_WAIT_TIMEOUT = 60.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sometimes loading a scene would take me slightly longer than 20 sec, and increasing the timeout seemed like it wouldn't cause problems. Let me know if you anticipate any issues.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is fine.
UNITY_STARTUP_WAIT_TIMEOUT = 10.0 | ||
IMAGE_COUNT = 500 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The maximum number of previous images to show. It's this large because if you "Run Required Actions" on some of the Set Rotation scenes (like 20
), it will run over 400 actions, and you might want to review images from all of the actions.
This quantity doesn't seem to cause bad performance. In fact, I think the main slowdown is time.sleep(0.5)
in _post_step_and_get_output
(though I'm sure that's there for a reason).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIRC, that time.sleep(0.5) had to do with making sure we have the most up-to-date images (we used to be a step behind), but we can revisit if it becomes an issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to know! Thanks!
return self.img_name, self.step_output | ||
return list_of_img_files[:image_count], self.step_output |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function now returns a list of the most recent (up to) 500 images, from newest to oldest.
if goal.get('action_list'): | ||
if len(goal.get('action_list', [])) > step_number: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug fix.
key = params["keypress"] | ||
action_string, img, step_output, action_list = mcs_interface.perform_action(params) # noqa: E501 | ||
img = convert_image_path(img) | ||
key = params["keypress"] if "keypress" in params else None | ||
action = params["action"] if "action" in params else None | ||
action_output = mcs_interface.perform_action(params) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're using most of the same functions for both the key press and the new "quick action" buttons.
'reward': output.reward, | ||
'reward': round(output.reward, 3), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was bothering me...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a couple comments but this looks good!
Was looking at my local webenabled image directory as I was testing -- would it be good to have a ticket that deletes some of those after 30 days or something? Just throwing that out there, I usually end up going through and manually removing some myself.
@rartiss55 There's a similar ticket, MCS-1926, for deleting files when the browser session ends, but we could rescope it to delete old files instead (I have no preference). |
@ThomasSchellenbergNextCentury Thanks, the alignment looks good now! And that existing ticket sounds fine to me. |
0967602
into
new-webenabled