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

hints feature #581

Merged
merged 3 commits into from
Sep 3, 2024
Merged

hints feature #581

merged 3 commits into from
Sep 3, 2024

Conversation

scooterbreak
Copy link
Contributor

@scooterbreak scooterbreak commented Jan 28, 2024

hints feature using wiki2vec pathfinding bot

All bot code taken from https://github.com/wikispeedruns/wikispeedruns-bot

Copy link
Collaborator

@dqian3 dqian3 left a comment

Choose a reason for hiding this comment

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

Looks like a great start! There are a few small things that I commented on below that we would need before we merge this in though.


sprint_api = Blueprint('sprints', __name__, url_prefix='/api/sprints')
# this script doesn't work
# !./get_embeddings.sh
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, so that is a jupyter notebook specific thing. In a really setting, we would just run this ourselves on the command line somewhere. Maybe modify this comment to say something about needing to download the embeddings.

# greedy = GreedySearch(embeddings_provider, graph_provider)
# path = greedy.search(start, end)

beam = BeamSearch(embeddings_provider, graph_provider)
Copy link
Collaborator

Choose a reason for hiding this comment

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

So here, what you should do instead of doing the whole search is do a single step, i.e. look at the pages linked from your current article, and then compare them with the embedding of the goal.

Doing a whole beam search is a lot of unnecessary work.

Note this would require modifying the wsbot library, which I can help with if you're intested. The best way I see of doing that is adding a separate get_link_rankings or something to the wsbot lib, that both the Search and external users (such as this API) can call.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah that makes sense I'll look into this. Thanks for reviewing!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dqian3 Sorry for the wait, instead of making a function for hints that returns rankings I just made one that iterates through the current page's links and returns the closest word based on a greedy approach. I added code for the greedy search to use this but left it commented out for now. Could you take a look when you get a chance?

@@ -57,6 +57,12 @@
Current Article<br><strong>[[currentArticle]]</strong>
</div>
</div>
<div style="float: right;" >
Copy link
Collaborator

Choose a reason for hiding this comment

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

This looks good!

@scooterbreak
Copy link
Contributor Author

  • Instead of calculating the whole path to the end article, I added a get_next_greedy_link function in search.py that iterates through the current page's links and finds the closest word to the end goal. Added code for greedy search to use the function but left it commented out for now
  • Hint text will reset after a new link is clicked

@bricehalder bricehalder requested a review from dqian3 July 9, 2024 07:31
@scooterbreak scooterbreak changed the title initial commit for hints feature hints feature Aug 8, 2024
Copy link
Collaborator

@bricehalder bricehalder left a comment

Choose a reason for hiding this comment

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

Looks great! This should surely not crash the server

@bricehalder bricehalder merged commit fa09000 into wikispeedruns:main Sep 3, 2024
1 check failed
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.

3 participants