sly icon indicating copy to clipboard operation
sly copied to clipboard

`slynk-completion::flex-score-1` is broken [or at least could follow Emacs's version]

Open SwiftLawnGnome opened this issue 4 years ago • 12 comments

Sorry I can't think of a more descriptive title, but just take a look at the definition:

(defun flex-score-1 (string-length indexes)
  (float
   (/ (length indexes)
      (* string-length
         (+ 1 (reduce #'+
                      (loop for i from 0
                            for (a b) on `(,-1
                                           ,@indexes
                                           ,string-length)
                            while b
                            unless (zerop (- b a 1))
                              sum (expt 1 *flex-score-falloff*) into holes
                              and sum (- b a 1) into hole-length)))))))

It makes no sense. That loop always returns nil, so this is the same as (float (/ (length indexes) string-length)), right?

And beyond that, isn't (expt 1 *flex-score-falloff*) basically just (coerce 1 (type-of *flex-score-falloff*))? And why is i in that loop?

For reference, here's the docstring:

Does the real work of FLEX-SCORE. Given that INDEXES is a list of integer position of characters in a string of length STRING-LENGTH, say how well these characters represent that STRING. There is a non-linear falloff with the distances between the indexes, according to FLEX-SCORE-FALLOFF. If that value is 2, for example, the indices '(0 1 2) on a 3-long string of is a perfect (100% match,) while '(0 2) on that same string is a 33% match and just '(1) is a 11% match.

I'm not sure how the implementation is actually supposed to work, but obviously this isn't it.

SwiftLawnGnome avatar Oct 01 '20 06:10 SwiftLawnGnome

Agree there's something odd in that expt 1 :facepalm: but does the loop really always return nil? Maybe, I haven't tested.
This function went through a lot of changes. Becasue it's been mostly working in my day to day and, until now, nobody really bothered with it, it's probably horribly buggy. But not so that it bothers anyone. I still get pretty decent flex guesses.

Anyway Zach, if you're looking for stuff to do here, know that the Elisp function flex-score-match-tightness and completion-pcm--hilit-commonality in Emacs, which I also wrote, are likely much less buggy and do exactly what I want to do here. So the task here is to migrate those developments to SLY, translating from Elisp to CL. Shouldn't be very hard. Here's a comment of the Elisp flex algorithm:

;; To understand how this works, consider these bad
;; ascii(tm) diagrams showing how the pattern "foo"
;; flex-matches "fabrobazo", "fbarbazoo" and
;; "barfoobaz":

;;      f abr o baz o
;;      + --- + --- +

;;      f barbaz oo
;;      + ------ ++

;;      bar foo baz
;;          +++

;; "+" indicates parts where the pattern matched.  A
;; "hole" in the middle of the string is indicated by
;; "-".  Note that there are no "holes" near the edges
;; of the string.  The completion score is a number
;; bound by ]0..1]: the higher the better and only a
;; perfect match (pattern equals string) will have
;; score 1.  The formula takes the form of a quotient.
;; For the numerator, we use the number of +, i.e. the
;; length of the pattern.  For the denominator, it
;; first computes
;;
;;     hole_i_contrib = 1 + (Li-1)^(1/tightness)
;;
;; , for each hole "i" of length "Li", where tightness
;; is given by `flex-score-match-tightness'.  The
;; final value for the denominator is then given by:
;;
;;    (SUM_across_i(hole_i_contrib) + 1) * len
;;
;; , where "len" is the string's length.

joaotavora avatar Oct 01 '20 09:10 joaotavora

does the loop really always return nil? Maybe, I haven't tested.

Yes since there's no return, it only accumulates into two variables and then terminates when the list is exhausted, discarding their values.

Becasue it's been mostly working in my day to day and, until now, nobody really bothered with it, it's probably horribly buggy. But not so that it bothers anyone. I still get pretty decent flex guesses.

Yeah that's why this is a bad issue title. I haven't actually encountered issues, but I use sly-simple-completions. I just meant that the function isn't doing what it's supposed to.

Anyway Zach, if you're looking for stuff to do here, know that the Elisp function flex-score-match-tightness and completion-pcm--hilit-commonality in Emacs, which I also wrote, are likely much less buggy and do exactly what I want to do here. So the task here is to migrate those developments to SLY, translating from Elisp to CL. Shouldn't be very hard.

Interesting, I'll take a look but math is not my strong suit.

SwiftLawnGnome avatar Oct 01 '20 20:10 SwiftLawnGnome

I just meant that the function isn't doing what it's supposed to.

And I'm telling it's not that bad, since I use it and nothing broken.

Anyway, I found what what happened. A spurious unintented hunk made it into a commit 4e195232db92334825a32d60f5fff4936766e695 of the same file, likely when I was trying to adapt to work like Emacs's. I've reverted to its previous incarnation.

joaotavora avatar Oct 01 '20 20:10 joaotavora

Interesting, I'll take a look but math is not my strong suit.

Heh, but there's really not much math involved ;-)

joaotavora avatar Oct 01 '20 20:10 joaotavora

Anyway, I found what what happened. A spurious unintented hunk made it into a commit 4e195232db92334825a32d60f5fff4936766e695 of the same file, likely when I was trying to adapt to work like Emacs's. I've reverted to its previous incarnation.

Aha, that explains that. Great!

Interesting, I'll take a look but math is not my strong suit.

Heh, but there's really not much math involved ;-)

That's just how bad my math is :upside_down:

But really, I'm more confused about how to translate completion-pcm--hilit-commonality into CL since it uses regexps and text properties. I'll take a closer look now that the commit was reverted. I'll probably need to better understand all the moving parts of the Slynk completion implementation before I can make headway.

SwiftLawnGnome avatar Oct 02 '20 01:10 SwiftLawnGnome

You can investigate all you want, of course.

But if you look closely you'll notice that what's at stake a length of a string (the string being the matching candidate you wish to score) and a list of numbers that are indexes on that string, and which specify where in the string the pattern matched. The actual pattern and the string don't matter anymore.

We just want to score say (3, (0,1,2)) very high (because it means that the 3-long pattern perfectly matched the candidate) and (3, (0)) lower (because it means the 1-long pattern matched just the first letter). Also when the length of the string is longer, say 9 and all the hits are wide apart, say (0,4,8), we probably want that to score lower than (0,1,2) or (6,7,8), depending on the "tightness" of course. That's what that Elisp comment is explaining when talking about the holes.

joaotavora avatar Oct 02 '20 08:10 joaotavora

I too use simple-completions and have always had problems with flex completions. Here is how flex behaves for me:

Before pressing tab: before

After pressing tab: after

So pretty unusable :) It's actually quite unfortunate because the best feature to hit Common Lisp in years (package-local nicknames) does not work with sly-simple-completions

mfiano avatar Dec 08 '20 06:12 mfiano

@mfiano, I don't get that. You know the drill. Provide a recipe from Emacs -Q that I can work on, otherwise it's just a a meaningless screenshot. Also, sly-flex-completions is a completion-providing backend, it works with company and with most other completion-showing frontends. Try some of those.

Also, what's at stake in this particular issue right now are scoring tweaks that don't affect your strange behaviour at all, so if you still have a problem, open a new issue. I.e. don't just read "flex" in the issue title and assume it is a bin for all issues flex-related.

Segregating stuff like this and separating issues makes life much easier on me. I'm very busy at the moment, and these little gestures help a lot.

joaotavora avatar Dec 08 '20 09:12 joaotavora

I apologize. I will refrain from commenting on issues I didn't create in the future. In my defense, I was researching some bugs of mine, stumbled on this thread, read it in full, hence the

I too use

and wondered why cl:in-package was not scoring above some really odd candidates, and thought it was relevant to the scoring function that seems wrong as the original poster pointed out. Again, I'm sorry.

mfiano avatar Dec 08 '20 09:12 mfiano

Michael Fiano [email protected] writes:

I apologize. I will refrain from commenting on issues I didn't create in the future. In my defense, I was researching some bugs of mine, stumbled on this thread, read it in full, hence the

No need to apologize! You can certainly comment in issues you didn't create, if you think it is related to the matter at hand. Maybe I didn't do a very good job of having an intelligible enough flow of comments or renaming the issue title so it would be clearer.

I too use

and wondered why cl:in-package was not scoring above some really odd candidates, and thought it was relevant to the scoring function that seems wrong as the original poster pointed out. Again, I'm sorry.

OK, I get it. Sorry if I sounded harsh. These kinds of things are easy to reproduce (or not) with Emacs -Q, here's how I debug completion stuff.

Navigate to a directory wher eyou have both SLY and Company checked out (Company's optional, but it's pretty popular). And then:

~/Source/Emacs/emacs/src/emacs -Q -l sly/sly-autoloads.el -L company-mode -l company -f global-company-mode -f sly

Wait a bit and you should have a REPL (that is if you have a "lisp" executable somewhere in your path)

You can also add -f sly-symbol-completion-mode which will turn off this mode which is on by default. This might be useful in testing SLY's completion system with fido-mode, for example.

João

joaotavora avatar Dec 08 '20 10:12 joaotavora

I just figured it out after some bisecting of my configuration, and indeed it is unrelated to this issue. It turns out, sly's company backend does not like other backends added.afterwards:

  (add-to-list 'company-backends '(company-capf company-files))

Everything works fine with this removed, even if my company is not as useful as it was elsewhere. I will open another issue if it bothers me.

mfiano avatar Dec 08 '20 10:12 mfiano

Michael Fiano [email protected] writes:

I just figured it out after some bisecting of my configuration, and indeed it is unrelated to this issue. It turns out, sly's company backend does not like other backends added.afterwards:

SLY doesn't have a company backend (anymore), if you have one you should remove it.

(add-to-list 'company-backends '(company-capf company-files))

This sounds like you're re-adding company-capf somehow into company backends. The company backends system is a mess, i don't mess with it. Setting it to company-capf solely and then working with Emacs's completion API is the sanest choice. All of thi sin my opinion of course.

Everything works fine with this removed, even if my company is not as useful as it was elsewhere. I will open another issue if it bothers me.

This "elsewhere pain" can probably be handled with buffer-local values of company-backends. But it's probably reasonable to

(add-hook 'sly-mode-hook (lambda () (setq-local company-backends '(company-capf))))

Or something like that.

joaotavora avatar Dec 08 '20 10:12 joaotavora