Skip to content
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

LT-21899: Parse Prioritizer look at following word as well as preceding word #315

Merged
merged 11 commits into from
Oct 27, 2024

Conversation

jtmaxwell3
Copy link
Contributor

@jtmaxwell3 jtmaxwell3 commented Oct 23, 2024

I added code that gathers statistics about how often each approved analysis occurs in the context of a previous word and in the context of a following word. This is used by GetContextScore in ComparePriorityCounts to order the analyses for a particular occurrence.


This change is Reviewable

Copy link

github-actions bot commented Oct 23, 2024

LCM Tests

    16 files  ±0      16 suites  ±0   2m 53s ⏱️ -1s
 2 828 tests +1   2 808 ✅ +1   20 💤 ±0  0 ❌ ±0 
11 260 runs  +4  11 081 ✅ +4  179 💤 ±0  0 ❌ ±0 

Results for commit d7b1b32. ± Comparison against base commit 93d7e90.

♻️ This comment has been updated with latest results.

@jasonleenaylor
Copy link
Contributor

src/SIL.LCModel/DomainServices/AnalysisGuessServices.cs line 342 at r1 (raw file):

		/// <param name="occurrence"></param>
		/// <returns></returns>
		private IAnalysis GetBestAnalysis(ContextCount counts, AnalysisOccurrence occurrence)

The code in this method looks suspicious. I'm not sure how only checking the previousWordForm could be correct. I still have more code to read, but this has me scratching my head at the moment.

@jtmaxwell3
Copy link
Contributor Author

The code is actually correct, although it does look suspicious. I will fix it.

Copy link
Contributor

@jasonleenaylor jasonleenaylor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 1 of 1 files at r2, 1 of 1 files at r3, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @jtmaxwell3)

@jtmaxwell3 jtmaxwell3 merged commit 80c9d96 into master Oct 27, 2024
5 checks passed
@jtmaxwell3 jtmaxwell3 deleted the LT-21899 branch October 27, 2024 04:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants