Fix rankers' implementation and indexing errors #320
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
As the implementation is not correct it is better to revisit or rewrite this in the future.
Before, we were dealing with a vector of FeatureVector objects,ie one FeatureVector per entry in the training set. Now have a separate vector per query in the training set. Before queryids were completely ignored For more info on why this change is needed look the implementation of ListNetRanker at https://www.microsoft.com/en-us/research/wp-content/ uploads/2016/02/tr-2007-40.pdf at page 5 and for the implementation of ListMleRanker look at http://icml2008.cs.helsinki.fi/papers/167.pdf page 6. If you look closely you will notice that the current implementation doesn't take query into account which is clearly wrong.
This change applies to both ListNetRanker and ListMleRanker. The motivation for Xavier initialization in Neural Networks is to initialize the weights of the network so that the neuron activation functions are not starting out in saturated or dead regions. In other words, we want to initialize the parameters with random values that are not “too small” and not “too large.”
Gradient being used to update the parameter per query is divided by the number of documents associated with the query else it will simply give more weightage to a query which has more documents associated with it.
In effect, a bias value allows you to shift the activation function to the left or right, which may be critical for successful learning.
This are due to changes made from fixing ranker implementations, fixing indexing errors, adding Xavier initialisation, adding normalisation of gradient and adding bias combined. This also fixes scorer test which was wrong earlier.
Open
|
Thanks for your work on this, and sorry for having dropped the ball on getting it reviewed and merged. We've since switched CI from travis-ci to GHA, so I've rebase your branch onto current master, updated the new CI to remove the libsvm stuff and opened a new PR: #324 Closing this, will review and try to actually get this merged via the new PR. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is a followup of #278 and attempts to resolve the remaining issues.