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

commit to Universal recommender #13

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

commit to Universal recommender #13

wants to merge 5 commits into from

Conversation

r4sn4
Copy link

@r4sn4 r4sn4 commented May 19, 2016

Features included -

  1. must include particular items in result
  2. return properties with items in result
  3. blacklist items containing particular property such as category, brand
  4. recommend products in particular price range
  5. merged code of price range with date range
  6. merged code of blacklisting items with blacklist category, brand

Aggregate query for above features is -

curl -H "Content-Type: application/json" -d '{ "user": "u1", "num": 4,"includeItems" : ["Iphone 4","Nexus"], "blacklistCategory" :["c1"] , "blacklistBrand": ["b1"],"blacklistItems" :["i1"]}' http://localhost:8000/queries.json

We are working on code to make it more generic.

priceRange: Option[PriceRange] = None,// optional before and after filter applied to a date field
blacklistItems: Option[List[String]] = None,
blacklistCategory: Option[List[String]] = None,
blacklistBrand: Option[List[String]] = None)
extends Serializable
Copy link

@pferrel pferrel May 26, 2016

Choose a reason for hiding this comment

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

We need to be able to specify a filter like we do with properties, which include the property name so they can be used for any property not just Category or Brand. I suggest an addition to the field definition, perhaps a bias enumeration, like

"bias": {
    "type": "not"
}

"bias": {
    "value": -1 // for filter
}

the type and value would not be used together.

by default the type is None, which means current behavior.

This will allow any number of fields to be set for any named property. Likewise this could be put in the engine.json for global application or into the query.

Copy link
Author

Choose a reason for hiding this comment

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

Hi Pat

Just read your suggestions. Will work on it.
Once completed will send you PR

@pferrel
Copy link

pferrel commented May 26, 2016

Some good work but we need to work with any property name not just Category or Brand, all properties are defined by the JSON in the $set so any name will work. Therefor the "field" in engine.json and the query will apply to them. So perhaps an extension of the "field" definition is the best way to handle categorical property exclusions.

Likewise for the numerical range, this needs to be renamable for times when it does not apply to "price"

@r4sn4
Copy link
Author

r4sn4 commented May 30, 2016

Hi Pat

Any suggestion on return Item properties along with Item score in recommendation result?

@pferrel
Copy link

pferrel commented May 30, 2016

These changes seem rather ambitious maybe create a new one for returning item properties against the master? That change is fairly simple but getting these working in a general way without hardcoded names will be more work.

The item properties are returned from Elasticsearch but not used to create the result list. You could change the PredictedResults to include more data along with the score. It should be an Option controlled by a flag in the query or engine.json. Something like "returnProperties": true, which defaults to false to work like it does now. This could be in the engine.json to work globally and in the query for use in the single results.

@r4sn4
Copy link
Author

r4sn4 commented May 31, 2016

Thanks for response Pat. Working on this

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.

2 participants