Skip to content
This repository was archived by the owner on Oct 31, 2023. It is now read-only.

Conversation

@Bowid
Copy link

@Bowid Bowid commented Sep 10, 2019

  1. Predict now use std::set instead priority_queue. From my point of
    view it is better to maintain set of fixed size and drop predictions
    with too low score immideately instead storing them in ordered manner.
  2. Implemented extension class of StarSpace to make things a bit more
    pythonic. It is definitely better to have return value instead of
    passing parameters to fill to functions. From my point of view, speed is
    not changed in case modern C++ compilers handle return of complex values
    properly without copying them.

@facebook-github-bot
Copy link

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

@facebook-github-bot
Copy link

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@pmirla
Copy link

pmirla commented Sep 30, 2019

Hello Bowid. I am also working on python wrapper. Can you please write back with your contacts to pavanmirla @gmail. Thanks

@Bowid
Copy link
Author

Bowid commented Sep 30, 2019

Hello Bowid. I am also working on python wrapper. Can you please write back with your contacts to pavanmirla @gmail. Thanks

Replied. Also here is the sample, let it be here

import starwrap

model_path = '/path/to/trained/model'

args = starwrap.args()
model = starwrap.starSpace(args)
model.initFromSavedModel(model_path)

line = input("Enter text for classification: ")
parsed = model.parseDoc(line, ' ')

# Output is list of tuples in format (probability, token_id)
# Predict takes two arguments: parsed doc and maximum predictions count
tokens = model.predict(parsed, 10)

probabilities = [item[0] for item in tokens]
# Token_id is integer, thus it should be rendered to feature (string)
features = model.renderTokens(tokens)

for feature, probability in zip(features, probabilities):
    print('Feature {f} probability {p}'.format(f=feature, p=probability))

@Bowid
Copy link
Author

Bowid commented Oct 3, 2019

I have some more thoughts to make things more pythonic, but it is nice to have feedback from FB guys and probably merge this PR to master.

Copy link
Contributor

@ledw ledw left a comment

Choose a reason for hiding this comment

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

Looks good over all. Thanks for contributing!
Please resolve the conflict with current version. Please run example tests to make sure this does not break current system, and add some test around the new predict function.

predict(input, pred, args_->K);
}

void StarSpace::predict(
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add some test around this function?

Copy link
Author

Choose a reason for hiding this comment

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

Whoops, unfortunately things goes wrong and i closed this PR after merging changes. So, the new one is here #279

Tests are coming soon

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants