Fine-tune HQP Candidate Result Scoring

Fine-tune HQP Candidate Result Scoring

BUG=111792
TEST=Adjusted and ran unit tests. Added new tests.
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=120346

[mrossetti, 2012-01-31 20:52:44]

Peter, thanks for the observations in the bug report. I've been looking forward
to doing some fine-tuning on the scoring and Mark's diagnostic page has made
'comparison scoring' so much easier.

I've taken out the outlier elimination code and gone to an averaging approach.
It's not awfully dissimilar to the original score calculation but I think it
much more consistent. It's now much easier to control the 'weighting' of
factors, so if, for example, we decide typed-count should be bumped a bit more
in importance it's one constant to change.

I manually tested with the profile you sent me and believe you will be pleased
with the results.

[Peter Kasting, 2012-01-31 21:13:57]

It seems like this change should either have new or modified unit tests, which
verify some of the ordering of URLs that you're fixing.  For example, we could
verify that (for input data that looked like the profile I mailed to you)
"techmeme.com", "teamliquid.net", and "teamliquid.net/tlpd" ranked in that
order.  (We wouldn't want to check exact scores, just overall ranking.)  This
test would be designed to fail before this change.

LGTM otherwise.

https://chromiumcodereview.appspot.com/9317011/diff/1/chrome/browser/history/...
File chrome/browser/history/url_index_private_data.cc (right):

https://chromiumcodereview.appspot.com/9317011/diff/1/chrome/browser/history/...
chrome/browser/history/url_index_private_data.cc:620: // 1/kMaxSignificantStart
points for each character later than the first at
Nit: I just realized the "1/" here should actually be removed.

[mrossetti, 2012-02-02 23:20:03]

I've added a new set of unit tests under the class HQPOrderingTest that test the
specific ordering suggestions you made Peter. Please let me know if there are
other tests like this that would be of value.

https://chromiumcodereview.appspot.com/9317011/diff/1/chrome/browser/history/...
File chrome/browser/history/url_index_private_data.cc (right):

https://chromiumcodereview.appspot.com/9317011/diff/1/chrome/browser/history/...
chrome/browser/history/url_index_private_data.cc:620: // 1/kMaxSignificantStart
points for each character later than the first at
I believe the comment is accurate. The calculation starting on the third line
following doesn't make the intent of the comment clear because other elements
have been folded in, such as the limiting performed by the min. In essence, the
statement is: deduct 1/50th (kMaxSignificantLength) of 1000 (kCompleteMaxValue)
per character for the distance at which the term first appears in the string, up
to a maximum distance of 50 characters.
 

On 2012/01/31 21:13:57, Peter Kasting wrote:
> Nit: I just realized the "1/" here should actually be removed.

[Peter Kasting, 2012-02-03 05:10:55]

LGTM, thanks for the tests.

https://chromiumcodereview.appspot.com/9317011/diff/1/chrome/browser/history/...
File chrome/browser/history/url_index_private_data.cc (right):

https://chromiumcodereview.appspot.com/9317011/diff/1/chrome/browser/history/...
chrome/browser/history/url_index_private_data.cc:620: // 1/kMaxSignificantStart
points for each character later than the first at
On 2012/02/02 23:20:03, mrossetti wrote:
> I believe the comment is accurate.

It isn't, based on your subsequent comments, although my suggestion was also
wrong.

> deduct 1/50th (kMaxSignificantLength) of 1000 (kCompleteMaxValue)

That would be "kStartMaxValue / kMaxSignificantStart points", not "1 /
kMaxSignificantStart points".

[mrossetti, 2012-02-03 16:34:43]

WIll commit shortly.

https://chromiumcodereview.appspot.com/9317011/diff/6001/chrome/browser/histo...
File chrome/browser/history/url_index_private_data.cc (right):

https://chromiumcodereview.appspot.com/9317011/diff/6001/chrome/browser/histo...
chrome/browser/history/url_index_private_data.cc:620: // 1/kMaxSignificantStart
points for each character later than the first at
Comment corrected, thanks!