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.

Sign up for GitHub

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

Remove feature possible values validation aside from type #44

Merged
merged 4 commits into from
Apr 20, 2020
Merged

Remove feature possible values validation aside from type #44

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7bd8ab8
Remove feature possible values validation aside from type
lgvital Apr 20, 2020
8526219
remove all possible values references + validate value is a string for C
lgvital Apr 20, 2020
11907d3
Remove possibleValues/delayedReward validation
lgvital Apr 20, 2020
59a07c1
Validate every value in pset array is string
lgvital Apr 20, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 13 additions & 37 deletions static/js/banditMl.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,16 @@ banditml.BanditAPI.prototype.getContext = function(experimentId) {
return this.getItemFromStorage(this.contextName(experimentId)) || {};
};

banditml.BanditAPI.prototype.isValidArray = function(arr, arrValueType) {
if (!Array.isArray(arr)) {
return false;
}
if (!arrValueType) {
return arr.every(v => { return typeof v === arrValueType; });
}
return true;
};

banditml.BanditAPI.prototype.validateAndFilterFeaturesInContext = function (context, contextValidation) {
const self = this;
let filteredFeatures = {};
Expand All @@ -244,7 +254,6 @@ banditml.BanditAPI.prototype.validateAndFilterFeaturesInContext = function (cont
if (featureExists) {
const value = context[featureName];
const featureSpec = contextValidation[featureName];
const possibleValues = featureSpec.possible_values;
const featureType = featureSpec.type;
try {
if (value == null) {
Expand All @@ -254,33 +263,14 @@ banditml.BanditAPI.prototype.validateAndFilterFeaturesInContext = function (cont
self.assert(typeof value === "number", `Feature ${featureName} is expected to be numeric, but ${value} of type ${valueType} was passed.`);
} else if (featureType === "C") {
self.assert(
Array.isArray(possibleValues),
`Feature ${featureName} is categorical, but its possible values is not an array. Update the model appropriately in Bandit ML.`
);
self.assert(
lgvital marked this conversation as resolved.
Show resolved Hide resolved
possibleValues.includes(context[featureName]),
`Value ${value} is not recognized among possible values for feature ${featureName}. Please update the possible values in Bandit ML.`
typeof value === "string",
`Feature ${featureName} is a categorical that expects a string, but ${value} is not a string.`
);
} else if (featureType === "P") {
self.assert(
Array.isArray(possibleValues),
`Feature ${featureName} is a product set, but its possible values is not an array. Update the model appropriately in Bandit ML.`
);
self.assert(
typeof value === "string" || Array.isArray(value),
typeof value === "string" || self.isValidArray(value, "string"),
`Feature ${featureName} is a product set that expects an array or string, but ${value} is not an array or string.`
);
if (Array.isArray(value)) {
self.assert(
value.every(val => possibleValues.includes(val)),
`${value} is not included in ${featureName}'s possible values ${possibleValues}.`
);
} else {
self.assert(
possibleValues.includes(value),
`${value} is not included in ${featureName}'s possible values ${possibleValues}.`
);
}
}
filteredFeatures[featureName] = value;
} catch (e) {
Expand Down Expand Up @@ -578,26 +568,12 @@ banditml.BanditAPI.prototype.logDecision = function(context, decisionResponse, e
});
};

banditml.BanditAPI.prototype.isDelayedReward = function(reward, experimentId) {
let contextValidation = this.getItemFromStorage(this.contextValidationKey(experimentId));
this.assert(contextValidation, "contextValidation is null, possibly from calling logReward without updating context.")
// if every key is in the possible choices (IDs), we categorize as delayed reward
return Object.keys(reward).every(key => contextValidation.choices.possible_values.includes(key));
};

banditml.BanditAPI.prototype.logReward = function(reward, experimentId, decision = null, decisionId = null) {
const headers = {
"Authorization": `ApiKey ${this.banditApikey}`
};
this.assert(
reward && typeof reward === "object", "Reward needs to be a non-empty object.");
if (this.isDelayedReward(reward, experimentId)) {
this.assert(decision === null, `decision needs to be null for delayed rewards.`);
this.assert(decisionId === null, `decisionId needs to be null for delayed rewards.`);
} else {
this.assert(decision && typeof decision === "string", `For immediate rewards, decision needs to be a single string ID. Got ${decision} instead.`);
this.assert(decisionId && typeof decisionId === "string", `For immediate rewards, decisionId needs to be a single string ID. Got ${decisionId} instead.`);
}
this.asyncPostRequest(this.banditLogRewardEndpoint, headers, {
decisionId: decisionId,
decision: decision,
Expand Down