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] Autogenerate form inputs and getPrediction code #38492

Closed
wants to merge 11 commits into from

Conversation

Erin007
Copy link
Contributor

@Erin007 Erin007 commented Jan 11, 2021

applab-prediction

For the user's most recently saved model, we'll pull the model details and autogenerate test value form input fields, grab the values from those input fields and pass them to the re-instantiated ML model to get a prediction back in the context of an App Lab app.

predictButton.textContent = 'Predict';
predictButton.id = modelData.name + '_predict';
});
};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The good news:
This code correctly retrieves the user's most recently saved machine learning model, and automatically generates the correct design elements based on the selected features of that model and plops them into the design mode visualizer.

The bad news:
1.) the design elements are re-created each time the app loads, so this code is obviously not being called in the right place/at the right time.
2.) the elements are all stacked on each other in the top left corner; we'll have to find a way to cleverly arrange them.

In addition to the 2 hurdles above, the next step is to generate the code that should accompany these design elements (getting the values from dropdowns etc). My lead so far on that front is Applab.appendToEditor(code), which coincidentally is what libraries use to generate blocks for the imported functions.

We'll also want to hide all of this behind and experiment flag and set up some trigger for when App Lab should auto-generate and when it shouldn't. To extend the query param idea, I think we could set it up similarly to enableMaker where when enableAI (or whatever the flag is) is on we'd do both the autogenerating and set up an additional section of the block toolbox with the AI blocks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updates:

1.) I now set and check a localStorage variable to prevent the code and design elements from duplicating on page reload.

2.) designMode.createElement takes X, Y parameters to set the element's starting position. Leveraging those parameters + a little math, makes a less cluttered layout without any overlaps. Positioning will still be problematic if there are more generated elements than will fit on a single screen; but that feels like a problem for future polishing.

3.) I put the autogen code behind the experiment flag already in use applab-ml; once we have a clearer plan for how users will selected models in App Lab, we can revisit if we'd like an additional param or other mechanisms for triggering element and code autogeneration.

Copy link
Member

Choose a reason for hiding this comment

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

re 1.) There is a risk that a student will change computer and return to the same app lab level, and won't have the same local storage, and will get the same code auto-generated again. I think we should rely on something more server-side than client-side to make sure we auto-generate only once.

To capture some of our offline discussion, there are a few proposed principles for auto-generation:

  • app lab should be responsible for modifying its level content, rather than ai lab
  • ai lab shouldn't be able to force an export to app lab because it might force an overwrite of a student's previous work

To that end, I'd suggest a model in which ai lab writes an identifier for the level (set via levelbuilder) to UserMlModel. App lab levels can then be tagged (in levelbuilder) to auto-generate using the most-recently-saved model with that same identifier. We'd use something similar to the "starter code" system to make sure that app lab only does this the first time, even if the student changes computer.

Some benefits of this system:

  • an app lab level can auto-generate with a model that was saved on any earlier level.
  • app lab can give helpful directions if there isn't a model saved with the appropriate identifier yet.
  • ai lab is agnostic about app lab's existence.

Copy link
Contributor

Choose a reason for hiding this comment

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

re:

the design elements are re-created each time the app loads, so this code is obviously not being called in the right place/at the right time

unless i'm misunderstanding what this method does, i think all of this code should be a react component like the other design elements in this directory (they all have very similar structure): https://github.com/code-dot-org/code-dot-org/tree/staging/apps/src/applab/designElements

paletteParams: ['model', 'testValues', 'callback'],
params: ['"myModel"', 'testValues', 'function (value) {\n \n}']
});
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Still TBD where this block should live, likely in it's own toolbox; but, there's open discussion about how close to keep objects to ML (e.g. should object blocks live with the other variables or are they scoped to only an ML toolbox) and now that we're using a generic getPrediction block will it be strange to have a toolbox with only one block in it, so punting for now, given that the block is behind an experiment flag anyway.

@Erin007 Erin007 changed the title [AiLab] App Lab finds the user's most recently saved model and autogenerates … [AiLab] Autogenerate form inputs and getPrediction code Jan 15, 2021
@Erin007 Erin007 marked this pull request as ready for review January 15, 2021 21:13
@Erin007 Erin007 requested review from breville and a team January 15, 2021 21:15
export const commands = {
async getPrediction(opts) {
return new Promise((resolve, reject) => {
$.ajax({
Copy link
Contributor

Choose a reason for hiding this comment

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

we should import jquery into this file to make sure it's available when this method is called

Copy link
Contributor

Choose a reason for hiding this comment

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

in the long term, we will need to request this data once and store it in memory because right now this will make 2 network requests every time the block is executed

@@ -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
Copy link
Contributor

Choose a reason for hiding this comment

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

if UserMlModel.where(user_id: current_user.id).last is nil, this line will throw an error. i'm not totally sure what behavior we want in this case, but returning early with empty JSON might be the most appropriate response?

Comment on lines 696 to +698
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'
Copy link
Contributor

Choose a reason for hiding this comment

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

for clarity, we can organize these routes as a collection under the resource :ml_models: https://guides.rubyonrails.org/routing.html#adding-collection-routes

predictButton.textContent = 'Predict';
predictButton.id = modelData.name + '_predict';
});
};
Copy link
Contributor

Choose a reason for hiding this comment

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

re:

the design elements are re-created each time the app loads, so this code is obviously not being called in the right place/at the right time

unless i'm misunderstanding what this method does, i think all of this code should be a react component like the other design elements in this directory (they all have very similar structure): https://github.com/code-dot-org/code-dot-org/tree/staging/apps/src/applab/designElements

@Erin007
Copy link
Contributor Author

Erin007 commented Jan 21, 2021

Closing in favor of #38664

@Erin007 Erin007 closed this Jan 21, 2021
@Erin007 Erin007 deleted the auto-gen-design-elements branch January 21, 2021 19:46
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.

None yet

3 participants