-
-
Notifications
You must be signed in to change notification settings - Fork 132
docs(web): start internal doc on SearchSpace design, requirements, and analysis 🚂 #15161
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
base: refactor/web/correction-heuristic-and-thresholding
Are you sure you want to change the base?
docs(web): start internal doc on SearchSpace design, requirements, and analysis 🚂 #15161
Conversation
User Test ResultsTest specification and instructions User tests are not required |
mcdurdin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This helps a lot in understanding the SearchSpace, SearchPath, SearchCluster types.
But I do have some questions and suggestions:
- It would help to describe the shape of these types (i.e. properties, methods, and particularly how Path differs from Cluster).
- Include a concrete example at the top, of a short series of key events + resulting SearchSpace types to illustrate how the types are used. This should be a common case rather than a pathological one!
- Even after reading, I am not really clear why SearchCluster exists; why do SearchPaths need grouping and how does this help?
- I am a little unclear on the names of the types - Space vs Path. Why is a Path an implementation of a Space?
- I am unclear how a SearchPath can 'extend' a SearchSpace given a SearchSpace is just an interface without implementation? Isn't the relationship between SearchPath and SearchSpace 'implements'?
- I guess a Cluster could be called a PathCluster or a PathGroup to clarify the relationship?
- It seems like a large part of the reason for these types is fat fingering at word boundaries. Is that right? It's never explictly stated, just obliquely when defining the problem.
Formatting nit: we generally wrap our .md files at 80 chars
web/src/engine/predictive-text/worker-thread/docs/search-spaces.md
Outdated
Show resolved
Hide resolved
web/src/engine/predictive-text/worker-thread/docs/search-spaces.md
Outdated
Show resolved
Hide resolved
web/src/engine/predictive-text/worker-thread/docs/search-spaces.md
Outdated
Show resolved
Hide resolved
web/src/engine/predictive-text/worker-thread/docs/search-spaces.md
Outdated
Show resolved
Hide resolved
web/src/engine/predictive-text/worker-thread/docs/search-spaces.md
Outdated
Show resolved
Hide resolved
web/src/engine/predictive-text/worker-thread/docs/search-spaces.md
Outdated
Show resolved
Hide resolved
web/src/engine/predictive-text/worker-thread/docs/search-spaces.md
Outdated
Show resolved
Hide resolved
web/src/engine/predictive-text/worker-thread/docs/search-spaces.md
Outdated
Show resolved
Hide resolved
web/src/engine/predictive-text/worker-thread/docs/search-spaces.md
Outdated
Show resolved
Hide resolved
web/src/engine/predictive-text/worker-thread/docs/search-spaces.md
Outdated
Show resolved
Hide resolved
web/src/engine/predictive-text/worker-thread/docs/search-spaces.md
Outdated
Show resolved
Hide resolved
web/src/engine/predictive-text/worker-thread/docs/search-spaces.md
Outdated
Show resolved
Hide resolved
|
After much reading and searching, I've landed on this: https://en.wikipedia.org/wiki/Modular_decomposition The new target form for the correction-search graph aligns pretty well with what is described there (after parsing all the formalization). To break it down:
Also of note: https://en.wikipedia.org/wiki/Quotient_graph (which is referenced by the prior link)
We can also build paths on the quotient graph, starting from the root node until the final module(s) added by the incoming keystroke, to somewhat formalize what the current Obvious remaining clarifications needed:
|
4888cbf to
0a4bc0b
Compare
…tion-search documentation
cb06ed9 to
dc1cd3b
Compare
|
I've now hoisted this PR much further up in the current chain, allowing it to serve somewhat as a design / implementation doc. |
mcdurdin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RSLGTM; I think the document needs more of an introduction but don't want the internal documentation to block merge
| There is one major, notable simplifying assumption in the current | ||
| text-correction design: we assume that each keystroke's `Transform` is 100% | ||
| independent from the `Transform` selected for every other keystroke. This | ||
| assumption is, of course, invalid: the output of keystroke A may selectively | ||
| establish the context needed for a Keyman keyboard rule matched by one or more | ||
| keys in keystroke B. Efforts to address this limitation are considered | ||
| out-of-scope at this time and will be addressed later in a future epic - | ||
| epic/true-correction - documented as issue #14709. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This assumption is, of course, invalid: the output of keystroke A may selectively
establish the context needed for a Keyman keyboard rule matched by one or more
keys in keystroke B.
This sentence hurts my brain. How can keystroke B have more than one key? That doesn't make sense. I guess what you are saying is that key events are not independent because rule matching depends on the output of previously executed rules?
This PR aims to start an internal doc on the role of
SearchSpace,SearchPath, andSearchClusterin the correction-search process.At present, I don't claim it to be complete by any measure. But, "something" is better than "nothing" here, and this provides a chance to get some eyes on things early in order to determine what works as an explanation and what doesn't. Feedback appreciated, even while in draft mode.
Build-bot: skip
Test-bot: skip