Fix implementation of rankers and indexing errors #278
Conversation
VaibhavKansagara
force-pushed
the
fix-rankers
branch
from
7856d00
to
b697dfa
Compare
There was a problem hiding this comment.
A good rule of thumb is if you end up writing "and" in your commit message, you should stop and think about whether there's too much in it. "Fixing things" isn't a single change — your main commit in this PR is actually three different changes.
You should also get rid of the machinery in configure.ac and elsewhere (such as .travis.yml at top level) to do with libsvm now we aren't dependent on it.
VaibhavKansagara
force-pushed
the
fix-rankers
branch
from
07377d7
to
19c43ba
Compare
|
The clang+libc++ build on Linux and the macOS build (which also uses clang+libc++) are both failing letor tests in travis-ci. I'm guessing this means there's some undefined behaviour being triggered somewhere, which happens to behave differently for GCC+libstdc++ vs clang+libc++. I'd try a build with asan (which is supported by both GCC and clang) and see if that flushes out any issues. |
|
sorry for the late fixups, I was busy with academic work. I will look into rest of the comments tomorrow morning. |
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.
VaibhavKansagara
force-pushed
the
fix-rankers
branch
from
5b56947
to
acc0586
Compare
|
The changes suggested are made #320. |
|
I've cherry-picked the first commit (the removal of SVMRanker) onto current master and pushed it along with an update to the new CI configuration (we've since switched from travis-ci to GHA). I'll resolve the rest in #324. |
No description provided.