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

[AiLab] Model Manager Dialog and autogenerated design elements and code #38664

Merged
merged 43 commits into from
Feb 15, 2021
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
5af4d1e
beginnings of a generic ml predict block
Erin007 Jan 6, 2021
4ed5c30
stop trying to use a dropdown for model names
Erin007 Jan 7, 2021
cd853b3
put prediction block behind applab-ml flag
Erin007 Jan 7, 2021
a018f1c
App Lab finds the user's most recently saved model and autogenerates …
Erin007 Jan 11, 2021
1a196bf
prepend design_ to ids, append onClick starter code
Erin007 Jan 12, 2021
fe30108
hack the dupe ids, layout design elements so they don't overlap
Erin007 Jan 13, 2021
74a8b96
merge in applab-model-load branch to access getPredict block
Erin007 Jan 13, 2021
7360835
get a prediction back from the trained model
Erin007 Jan 15, 2021
c2b38fc
tidy autogen code to prevent App Lab warnings
Erin007 Jan 15, 2021
1b931ab
put autogen function call behind experiment flag
Erin007 Jan 15, 2021
5387166
use localStorage to prevent re-gen of design elements and code on pag…
Erin007 Jan 15, 2021
8ee19f7
add manage models as an option in the cog menu
Erin007 Jan 19, 2021
a4ad3dc
ModelManagerDialog component open/close
Erin007 Jan 20, 2021
a693b39
Import button that autocreates starter code and design elements
Erin007 Jan 20, 2021
db7ce74
dropdown to select model to import
Erin007 Jan 20, 2021
14a6519
remove references to the allowAutoGenElements dead end boolean
Erin007 Jan 21, 2021
568be9e
Merge branch 'staging' into model-manager
Erin007 Feb 1, 2021
a92018d
hoist spacing pixel amount into variable
Erin007 Feb 1, 2021
26a9139
import jquery
Erin007 Feb 1, 2021
c561c6b
rely on model_id, don't grab the user's most recent model
Erin007 Feb 1, 2021
4425ebc
disable import button if the user doesn't have any trained models
Erin007 Feb 1, 2021
05f8cca
pass model id as a param to handle duplicate names and reduce ajax calls
Erin007 Feb 1, 2021
4354b6c
spinner and async for autogen code and elements
Erin007 Feb 2, 2021
f2f07e5
Merge branch 'staging' into model-manager
Erin007 Feb 2, 2021
6fd6500
lazy load the ModelManagerDialog
Erin007 Feb 3, 2021
72a2943
move autogen function into standalone file to keep applab.js tidier a…
Erin007 Feb 3, 2021
57878c8
fix SettingsCogTest by removing loabable to avoid the spinner icon
Erin007 Feb 4, 2021
2348784
stop using fat arrow to maybe? appease Uglify
Erin007 Feb 4, 2021
30d1396
pass autogenerateML down through the component tree
Erin007 Feb 4, 2021
7b91f3b
transpile all of the ml packages that have lib and lib-es6
Feb 9, 2021
52cf24b
don't require func
Erin007 Feb 9, 2021
15f5245
debugging thumbnail generation
Erin007 Feb 10, 2021
bf54c02
debugging thumbnails
Erin007 Feb 10, 2021
c313d29
Merge branch 'staging' into model-manager
breville Feb 11, 2021
90a6371
clean up logs, un-skip tests, only render modelmanager dialog if expe…
Erin007 Feb 11, 2021
5a422f1
allow non-owners to access models in shared projects, protect against…
Erin007 Feb 12, 2021
5144b3d
seperate UI generation from ajax call
Erin007 Feb 12, 2021
c2c4983
reduce # of times ml_model/names is called
Erin007 Feb 12, 2021
96f869c
surface failure in app lab
Erin007 Feb 13, 2021
d2e4a97
remove changes to sharedialog
Erin007 Feb 13, 2021
3827a44
remove console.log
Erin007 Feb 13, 2021
bea44e7
remove changes to sharealloweddialog
Erin007 Feb 13, 2021
66b4898
remove stray change
Erin007 Feb 13, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 6 additions & 4 deletions apps/src/MLTrainers.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const KNN = require('ml-knn');
import KNN from 'ml-knn';

const KNNTrainers = ['knnClassify', 'knnRegress'];

Expand Down Expand Up @@ -31,18 +31,20 @@ modelData = {
}

value in testData is the converted algorithm-ready number, not the string

TODO: convert string data in testValues using the featureNumberKey
*/
export function predict(modelData) {
// Determine which algorithm to use.
if (KNNTrainers.includes(modelData.selectedTrainer)) {
// Re-instantiate the trained model.
const model = KNN.load(modelData.trainedModel);
// Prepare test data.
const testValues = modelData.selectedFeatures.map(
feature => modelData.testData[feature]
const testValues = modelData.selectedFeatures.map(feature =>
parseInt(modelData.testData[feature])
);
// Make a prediction.
const rawPrediction = model.predict(testValues)[0];
const rawPrediction = model.predict(testValues);
// Convert prediction to human readable (if needed)
const prediction = Object.keys(modelData.featureNumberKey).includes(
modelData.labelColumn
Expand Down
8 changes: 8 additions & 0 deletions apps/src/applab/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -534,3 +534,11 @@ export function drawChartFromRecords(
callback: callback
});
}

export function getPrediction(model, testValues, callback) {
return Applab.executeCmd(null, 'getPrediction', {
model,
testValues,
callback
});
}
58 changes: 58 additions & 0 deletions apps/src/applab/applab.js
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,64 @@ function renderFooterInSharedGame() {
);
}

Applab.autogenerateML = function(modelId) {
$.ajax({
url: `/api/v1/ml_models/${modelId}`,
method: 'GET'
}).then(modelData => {
Erin007 marked this conversation as resolved.
Show resolved Hide resolved
var x = 20;
var y = 20;
designMode.onInsertEvent(`var testValues = {};`);
modelData.selectedFeatures.forEach(feature => {
y = y + 20;
Erin007 marked this conversation as resolved.
Show resolved Hide resolved
var label = designMode.createElement('LABEL', x, y);
label.textContent = feature + ':';
label.id = 'design_' + feature + '_label';
label.style.width = '300px';
y = y + 20;
if (Object.keys(modelData.featureNumberKey).includes(feature)) {
var selectId = feature + '_dropdown';
var select = designMode.createElement('DROPDOWN', x, y);
select.id = 'design_' + selectId;
// App Lab automatically addss "option 1" and "option 2", remove them.
select.options.remove(0);
select.options.remove(0);
Object.keys(modelData.featureNumberKey[feature]).forEach(option => {
var optionElement = document.createElement('option');
optionElement.text = option;
select.options.add(optionElement);
});
} else {
var input = designMode.createElement('TEXT_INPUT');
input.id = 'design_' + feature + '_input';
}
var addFeature = `testValues.${feature} = getText("${selectId}");`;
designMode.onInsertEvent(addFeature);
});
y = y + 40;
var label = designMode.createElement('LABEL', x, y);
label.textContent = modelData.labelColumn;
// TODO: this could be problematic if the name isn't formatted appropriately
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Names with whitespace cause this warning in App Lab when the blocks are generated. WARNING: Line: 2: The onEvent() id parameter refers to an id ("did this save?_predict") which contains whitespace. Change the id name to ("didthissave?_predict")
We could either 1.) strip whitespace when we use the name to generate ids for UI elements OR 2.) strip whitespace from names on save from AI Lab. I'm leaning toward option 1. Still need to see what happens with special characters, 😄 etc.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are the column names? We could also remove spaces way back at import time into AI Lab? And our precanned datasets could just be appropriately named already? It would probably make all of our code safer, since I think AI Lab uses these column names for object keys too..

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are model names that the user inputs. Then I was using the model name to name UI elements, like on the line below. I guess we could 3.) use model id instead e.g. label.id = 'design_' + modelData.id + '_label';

label.id = 'design_' + modelData.name + '_label';
label.style.width = '300px';
y = y + 20;
var predictionId = modelData.name + '_prediction';
var prediction = designMode.createElement('TEXT_INPUT', x, y);
prediction.id = 'design_' + predictionId;
y = y + 40;
var predictButton = designMode.createElement('BUTTON', x, y);
predictButton.textContent = 'Predict';
var predictButtonId = modelData.name + '_predict';
designMode.updateProperty(predictButton, 'id', predictButtonId);
var predictOnClick = `onEvent("${predictButtonId}", "click", function() {
getPrediction("${modelData.name}", testValues, function(value) {
setText("${predictionId}", value);
});
});`;
designMode.onInsertEvent(predictOnClick);
});
};

Erin007 marked this conversation as resolved.
Show resolved Hide resolved
/**
* @param {string} code The code to search for Data Storage APIs
* @return {boolean} True if the code uses any data storage APIs
Expand Down
2 changes: 2 additions & 0 deletions apps/src/applab/commands.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
} from '../lib/util/javascriptMode';
import {commands as audioCommands} from '@cdo/apps/lib/util/audioApi';
import {commands as timeoutCommands} from '@cdo/apps/lib/util/timeoutApi';
import {commands as mlCommands} from '@cdo/apps/lib/util/mlApi';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see a lot of "ml" related naming, but it sounds like we are going to trend towards "ai" instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah - it'd be good to get all on the same page about that and update the code to match. Ok to do as a rename/refactor after this PR?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds fine.

import * as makerCommands from '@cdo/apps/lib/kits/maker/commands';
import {getAppOptions} from '@cdo/apps/code-studio/initApp/loadApp';
import {AllowedWebRequestHeaders} from '@cdo/apps/util/sharedConstants';
Expand Down Expand Up @@ -2291,4 +2292,5 @@ function stopLoadingSpinnerFor(elementId) {
// Include playSound, stopSound, etc.
Object.assign(applabCommands, audioCommands);
Object.assign(applabCommands, timeoutCommands);
Object.assign(applabCommands, mlCommands);
Object.assign(applabCommands, makerCommands);
11 changes: 11 additions & 0 deletions apps/src/applab/dropletConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
} from './setPropertyDropdown';
import {getStore} from '../redux';
import * as applabConstants from './constants';
import experiments from '../util/experiments';

var DEFAULT_WIDTH = applabConstants.APP_WIDTH.toString();
var DEFAULT_HEIGHT = (
Expand Down Expand Up @@ -1131,6 +1132,16 @@ export var blocks = [
}
];

if (experiments.isEnabled(experiments.APPLAB_ML)) {
blocks.push({
func: 'getPrediction',
parent: api,
category: 'Data',
paletteParams: ['model', 'testValues', 'callback'],
params: ['"myModel"', 'testValues', 'function (value) {\n \n}']
});
}

export const categories = {
'UI controls': {
id: 'uicontrols',
Expand Down
67 changes: 67 additions & 0 deletions apps/src/code-studio/components/ModelManagerDialog.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import PropTypes from 'prop-types';
import React from 'react';
import BaseDialog from '@cdo/apps/templates/BaseDialog';
import Button from '@cdo/apps/templates/Button';
import Applab from '@cdo/apps/applab/applab';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if this is the cause of your storybook problems, but it seems like the code you are adding should not be relying on applab like this. Looking at the chain of dependencies, this is going to make it so that every level type which uses CodeWorkspace is now going to be importing all of App Lab. One idea would be to break this dependency, and then see if you still have the same storybook problems. Please let me know if you need more info on how to accomplish this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess I'll offer some info anyway :-) One way would be to "lazy load" the ModelManagerDialog, like this:

const ImagePicker = loadable(() => import('../components/ImagePicker'));

there may also be a simpler way to cut out the dependencies, but it might take a bit more understanding of your code than I currently have to figure out where.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aha! yes, this import of Applab does seem to be the culprit, though I still don't understand the full intricacies of how. Thank you! I'll give lazy loading a try.


export default class ModelManagerDialog extends React.Component {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this dialog will live next to and be most similar to these two components, so just linking those here in case it's helpful:
<LibraryManagerDialog/>
<AssetManager/>

The LibraryManagerDialog is newer and more consistent with our newer styles, so i'd go with that one if you're trying to decide how to style something (related to our convo here)

static propTypes = {
isOpen: PropTypes.bool.isRequired,
onClose: PropTypes.func.isRequired
};

state = {
models: []
};

componentDidUpdate() {
this.getModelList();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When running a regular AppLab app, we keep on hitting /api/v1/ml_models/names. This component seems to be updating more than expected.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

componentDidMount() instead of componentDidUpdate() 🤦‍♀️

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That seems to fire once, but due to the lifetime management of these dialogs (they live forever, just showing and hiding), it doesn't refresh the model list each time the dialog is opened. I have implemented what seems to be a solution in #39041.

}

closeModelManager = () => {
this.props.onClose();
};

getModelList = () => {
$.ajax({
url: '/api/v1/ml_models/names',
method: 'GET'
}).then(models => {
this.setState({models});
});
};

importMLModel = () => {
const modelId = this.root.value;
Applab.autogenerateML(modelId);
};

render() {
const {isOpen} = this.props;

return (
<div>
<BaseDialog
isOpen={isOpen}
handleClose={this.closeModelManager}
useUpdatedStyles
>
<h2>Machine Learning Models</h2>
<select name="model" ref={element => (this.root = element)}>
{this.state.models.map(model => (
<option key={model.id} value={model.id}>
{model.name}
</option>
))}
</select>
<Button
text={'Import'}
color={Button.ButtonColor.orange}
onClick={this.importMLModel}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this function does most of its work asynchronously, should we show a spinner or something similar while it does its work?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screen.Recording.2021-02-01.at.9.29.13.PM.mov

/>
<h3>Model card details will go here.</h3>
</BaseDialog>
</div>
);
}
}
22 changes: 21 additions & 1 deletion apps/src/lib/ui/SettingsCog.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import PopUpMenu from './PopUpMenu';
import ConfirmEnableMakerDialog from './ConfirmEnableMakerDialog';
import LibraryManagerDialog from '@cdo/apps/code-studio/components/libraries/LibraryManagerDialog';
import {getStore} from '../../redux';
import experiments from '@cdo/apps/util/experiments';
import ModelManagerDialog from '@cdo/apps/code-studio/components/ModelManagerDialog';

const style = {
iconContainer: {
Expand Down Expand Up @@ -55,7 +57,8 @@ class SettingsCog extends Component {
open: false,
canOpen: true,
confirmingEnableMaker: false,
managingLibraries: false
managingLibraries: false,
managingModels: false
};

open = () => this.setState({open: true, canOpen: false});
Expand All @@ -77,6 +80,11 @@ class SettingsCog extends Component {
this.setState({managingLibraries: true});
};

manageModels = () => {
this.close();
this.setState({managingModels: true});
};

toggleMakerToolkit = () => {
this.close();
if (!makerToolkitRedux.isEnabled(getStore().getState())) {
Expand All @@ -97,6 +105,7 @@ class SettingsCog extends Component {
showConfirmation = () => this.setState({confirmingEnableMaker: true});
hideConfirmation = () => this.setState({confirmingEnableMaker: false});
closeLibraryManager = () => this.setState({managingLibraries: false});
closeModelManager = () => this.setState({managingModels: false});

setTargetPoint(icon) {
if (!icon) {
Expand Down Expand Up @@ -145,10 +154,17 @@ class SettingsCog extends Component {
{this.areLibrariesEnabled() && (
<ManageLibraries onClick={this.manageLibraries} />
)}
{experiments.isEnabled(experiments.APPLAB_ML) && (
<ManageModels onClick={this.manageModels} />
)}
{this.props.showMakerToggle && (
<ToggleMaker onClick={this.toggleMakerToolkit} />
)}
</PopUpMenu>
<ModelManagerDialog
isOpen={this.state.managingModels}
onClose={this.closeModelManager}
/>
<ConfirmEnableMakerDialog
isOpen={this.state.confirmingEnableMaker}
handleConfirm={this.confirmEnableMaker}
Expand All @@ -173,6 +189,10 @@ ManageAssets.propTypes = {
last: PropTypes.bool
};

export function ManageModels(props) {
return <PopUpMenu.Item {...props}>{'Manage Models'}</PopUpMenu.Item>;
}

export function ManageLibraries(props) {
return <PopUpMenu.Item {...props}>{msg.manageLibraries()}</PopUpMenu.Item>;
}
Expand Down
32 changes: 32 additions & 0 deletions apps/src/lib/util/mlApi.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/* global Promise */

import {predict} from '@cdo/apps/MLTrainers';

export const commands = {
async getPrediction(opts) {
return new Promise((resolve, reject) => {
$.ajax({
url: '/api/v1/ml_models/names',
method: 'GET'
})
.then(data => {
const modelId = data.find(model => model.name === opts.model).id;
$.ajax({
url: '/api/v1/ml_models/' + modelId,
method: 'GET'
}).then(modelData => {
const predictParams = {
...modelData,
testData: opts.testValues
};
const result = predict(predictParams);
opts.callback(result);
});
return resolve();
})
.fail((jqXhr, status) => {
return reject({message: 'An error occurred'});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it does seem we might want to expose this error to the user's program, so they can then report it back to their end-user.

});
});
}
};
5 changes: 3 additions & 2 deletions dashboard/app/controllers/api/v1/ml_models_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ def save
)
upload_to_s3(model_id, params["ml_model"].to_json)

render json: "hooray!"
render json: {id: model_id}
end

# GET api/v1/ml_models/names
Expand All @@ -31,7 +31,8 @@ def user_ml_model_names
# GET api/v1/ml_models/:model_id
# Retrieve a trained ML model from S3
def get_trained_model
model = download_from_s3(params[:model_id])
model_id = params[:model_id] ? params[:model_id] : UserMlModel.where(user_id: current_user.id).last.model_id
Erin007 marked this conversation as resolved.
Show resolved Hide resolved
model = download_from_s3(model_id)
render json: model
end

Expand Down
2 changes: 1 addition & 1 deletion dashboard/config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -695,7 +695,7 @@

post 'ml_models/save', to: 'ml_models#save'
get 'ml_models/names', to: 'ml_models#user_ml_model_names'
get 'ml_models/:model_id', to: 'ml_models#get_trained_model'
get 'ml_models(/:model_id)', to: 'ml_models#get_trained_model'

resources :teacher_feedbacks, only: [:index, :create] do
collection do
Expand Down
1 change: 1 addition & 0 deletions lib/cdo/shared_constants.rb
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,7 @@ module SharedConstants
"getUserId": null,
"drawChart": null,
"drawChartFromRecords": null,
"getPrediction": null,

// Turtle
"moveForward": null,
Expand Down