Skip to content

Merge equivalent MSAs when loading HermitCrab #1148

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.

Sign up for GitHub

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

Merged
jtmaxwell3 merged 8 commits into from
Oct 1, 2026
Merged

Merge equivalent MSAs when loading HermitCrab #1148

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
fd51700
Add UniqueStemMSAs
jtmaxwell3 Sep 21, 2026
c6b7755
Fixes bugs reported by John Lambert
jtmaxwell3 Sep 28, 2026
cc961f3
Add unit test
jtmaxwell3 Sep 29, 2026
05b26fe
Add MergeMSAs to HC parser parameters
jtmaxwell3 Sep 29, 2026
edded8e
Fix unit test
jtmaxwell3 Sep 29, 2026
ca68e83
Fix bugs reported by John Lambert
jtmaxwell3 Sep 30, 2026
73c25ef
Fix problems reported by Jason, add RepresentativeSAHvo
jtmaxwell3 Oct 1, 2026
d200abc
Add acceptance test requested by John Lambert
jtmaxwell3 Oct 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 58 additions & 9 deletions Src/LexText/ParserCore/HCLoader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,17 @@ public static Language Load(LcmCache cache, IHCLoadErrorLogger logger)
return loader.m_language;
}

public static Language Load(
LcmCache cache,
IHCLoadErrorLogger logger,
out Dictionary<IMoMorphSynAnalysis, IMoMorphSynAnalysis> representativeMSAs)
{
var loader = new HCLoader(cache, logger, true);
loader.LoadLanguage();
representativeMSAs = loader.m_representativeMSAs;
return loader.m_language;
}

private static readonly string[] VariableNames =
{
"α", "β", "γ", "δ", "ε", "ζ", "η", "θ", "ι", "κ", "λ", "μ", "ν", "ξ",
Expand All @@ -43,6 +54,7 @@ public static Language Load(LcmCache cache, IHCLoadErrorLogger logger)
private readonly LcmCache m_cache;
protected readonly Dictionary<IMoForm, List<Allomorph>> m_allomorphs;
private readonly Dictionary<IMoMorphSynAnalysis, List<Morpheme>> m_morphemes;
protected readonly Dictionary<IMoMorphSynAnalysis, IMoMorphSynAnalysis> m_representativeMSAs;
private readonly Dictionary<IMoStemName, StemName> m_stemNames;
protected readonly Dictionary<ICmObject, MprFeature> m_mprFeatures;

Expand Down Expand Up @@ -72,12 +84,13 @@ public static Language Load(LcmCache cache, IHCLoadErrorLogger logger)
private CharacterDefinition m_morphBdry;
protected static char m_ReplaceSpaceChar = '.';

protected HCLoader(LcmCache cache, IHCLoadErrorLogger logger)
protected HCLoader(LcmCache cache, IHCLoadErrorLogger logger, bool mergeMSAs = false)
{
m_cache = cache;
m_logger = logger;
m_allomorphs = new Dictionary<IMoForm, List<Allomorph>>();
m_morphemes = new Dictionary<IMoMorphSynAnalysis, List<Morpheme>>();
m_representativeMSAs = mergeMSAs ? new Dictionary<IMoMorphSynAnalysis, IMoMorphSynAnalysis>() : null;
m_stemNames = new Dictionary<IMoStemName, StemName>();
m_mprFeatures = new Dictionary<ICmObject, MprFeature>();

Expand Down Expand Up @@ -636,24 +649,60 @@ protected virtual void LoadLexEntries(Stratum stratum, ILexEntry entry, IList<IM
var mainEntry = component as ILexEntry;
if (mainEntry != null)
{
foreach (IMoStemMsa msa in mainEntry.MorphoSyntaxAnalysesOC.OfType<IMoStemMsa>())
foreach (IMoStemMsa msa in UniqueMSAs(mainEntry.MorphoSyntaxAnalysesOC.OfType<IMoStemMsa>()))
LoadLexEntryOfVariant(stratum, inflType, msa, allos, entry.ShortName);
}
else
{
ILexSense sense = (ILexSense)component;
if (sense != null && sense.MorphoSyntaxAnalysisRA is IMoStemMsa)
LoadLexEntryOfVariant(stratum, inflType, (IMoStemMsa)sense.MorphoSyntaxAnalysisRA, allos, entry.ShortName);
LoadLexEntryOfVariant(
stratum,
inflType,
(IMoStemMsa)RepresentativeMSA(sense.MorphoSyntaxAnalysisRA),
allos,
entry.ShortName);
}
}
}
}
}

foreach (IMoStemMsa msa in entry.MorphoSyntaxAnalysesOC.OfType<IMoStemMsa>())
foreach (IMoStemMsa msa in UniqueMSAs(entry.MorphoSyntaxAnalysesOC.OfType<IMoStemMsa>()))
LoadLexEntry(stratum, msa, allos, entry.ShortName);
}

IMoMorphSynAnalysis RepresentativeMSA(IMoMorphSynAnalysis msa)
{
if (m_representativeMSAs == null)
{
return msa;
}
if (msa is IMoDerivStepMsa)
{
return msa;
}
if (m_representativeMSAs.TryGetValue(msa, out IMoMorphSynAnalysis representativeMSA))
{
return representativeMSA;
}
ILexEntry entry = msa.OwnerOfClass<ILexEntry>();
foreach (var msa2 in entry.MorphoSyntaxAnalysesOC)
{
if (msa2.ClassID == msa.ClassID && msa2.EqualsMsa(msa) && entry.SenseWithMsa(msa2) != null)
{
m_representativeMSAs[msa] = msa2;
return msa2;
}
}
return msa;
}

private IEnumerable<T> UniqueMSAs<T>(IEnumerable<T> msas) where T : class, IMoMorphSynAnalysis
{
return msas.Select(msa => (T)RepresentativeMSA(msa)).Distinct();
}

protected IEnumerable<ILexEntryInflType> GetInflTypes(ILexEntryRef lexEntryRef)
{
if (lexEntryRef.VariantEntryTypesRS.Count == 0)
Expand Down Expand Up @@ -858,19 +907,19 @@ protected virtual void LoadMorphologicalRules(Stratum stratum, ILexEntry entry,
var mainEntry = component as ILexEntry;
if (mainEntry != null)
{
foreach (IMoMorphSynAnalysis msa in mainEntry.MorphoSyntaxAnalysesOC)
foreach (IMoMorphSynAnalysis msa in UniqueMSAs(mainEntry.MorphoSyntaxAnalysesOC))
LoadMorphologicalRule(stratum, entry, allos, msa);
}
else
{
var sense = (ILexSense)component;
LoadMorphologicalRule(stratum, entry, allos, sense.MorphoSyntaxAnalysisRA);
LoadMorphologicalRule(stratum, entry, allos, RepresentativeMSA(sense.MorphoSyntaxAnalysisRA));
}
}
}
}

foreach (IMoMorphSynAnalysis msa in entry.MorphoSyntaxAnalysesOC)
foreach (IMoMorphSynAnalysis msa in UniqueMSAs(entry.MorphoSyntaxAnalysesOC))
LoadMorphologicalRule(stratum, entry, allos, msa);
}

Expand Down Expand Up @@ -2213,10 +2262,10 @@ private IEnumerable<T[]> Permute<T>(List<List<T>> items, int index)
private void LoadMorphemeCoOccurrenceRules(IMoMorphAdhocProhib morphAdhocProhib)
{
List<Morpheme> firstMorphemes;
if (m_morphemes.TryGetValue(morphAdhocProhib.FirstMorphemeRA, out firstMorphemes))
if (m_morphemes.TryGetValue(RepresentativeMSA(morphAdhocProhib.FirstMorphemeRA), out firstMorphemes))
{
var allOthers = new List<List<Morpheme>>();
foreach (IMoMorphSynAnalysis msa in morphAdhocProhib.RestOfMorphsRS)
foreach (IMoMorphSynAnalysis msa in UniqueMSAs(morphAdhocProhib.RestOfMorphsRS))
{
List<Morpheme> hcMorphemes;
if (m_morphemes.TryGetValue(msa, out hcMorphemes))
Expand Down
59 changes: 46 additions & 13 deletions Src/LexText/ParserCore/HCParser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,14 @@
// This software is licensed under the LGPL, version 2.1 or later
// (http://www.gnu.org/licenses/lgpl-2.1.html)

using SIL.Data;
using SIL.LCModel;
using SIL.LCModel.Infrastructure;
using SIL.Machine.Annotations;
using SIL.Machine.Morphology.HermitCrab;
using SIL.Machine.Morphology.HermitCrab.MorphologicalRules;
using SIL.Machine.Rules;
using SIL.ObjectModel;
using System;
using System.Collections.Generic;
using System.Globalization;
Expand All @@ -10,13 +18,6 @@
using System.Text;
using System.Xml;
using System.Xml.Linq;
using SIL.LCModel;
using SIL.LCModel.Infrastructure;
using SIL.Machine.Annotations;
using SIL.Machine.Morphology.HermitCrab;
using SIL.Machine.Morphology.HermitCrab.MorphologicalRules;
using SIL.Machine.Rules;
using SIL.ObjectModel;

namespace SIL.FieldWorks.WordWorks.Parser
{
Expand All @@ -25,12 +26,14 @@ public class HCParser : DisposableBase, IParser
private readonly LcmCache m_cache;
private Morpher m_morpher;
private Language m_language;
private Dictionary<IMoMorphSynAnalysis, IMoMorphSynAnalysis> m_representativeMSAs;
Comment thread
jtmaxwell3 marked this conversation as resolved.
private readonly FwXmlTraceManager m_traceManager;
private readonly string m_outputDirectory;
private ParserModelChangeListener m_changeListener;
private bool m_forceUpdate;
private bool m_guessRoots;
private bool m_mergeAnalyses;
private bool m_mergeMSAs = true;

// the public const strings are for GenerateHCConfigForFLExTrans and HCSynthByGlossLib
internal const string CRuleID = "ID";
Expand Down Expand Up @@ -109,7 +112,8 @@ public ParseResult ParseWord(string word)
if (GetMorphs(wordAnalysis, out morphs))
{
analyses.Add(new ParseAnalysis(morphs.Select(mi =>
new ParseMorph(mi.Form, mi.Msa, mi.InflType, mi.GuessedString))));
new ParseMorph(mi.Form, mi.Msa, mi.InflType, mi.GuessedString)),
m_representativeMSAs));
}
}
result = new ParseResult(analyses);
Expand Down Expand Up @@ -155,25 +159,36 @@ private void LoadParser()
using (XmlWriter writer = XmlWriter.Create(loadErrorsFile))
using (new WorkerThreadReadHelper(m_cache.ServiceLocator.GetInstance<IWorkerThreadReadHandler>()))
{
writer.WriteStartElement("LoadErrors");
m_language = HCLoader.Load(m_cache, new XmlHCLoadErrorLogger(writer));
writer.WriteEndElement();
XElement parserParamsElem = XElement.Parse(m_cache.LanguageProject.MorphologicalDataOA.ParserParameters);
XElement delReappsElem = parserParamsElem.Elements("HC").Elements("DelReapps").FirstOrDefault();
XElement guessRootsElem = parserParamsElem.Elements("HC").Elements("GuessRoots").FirstOrDefault();
XElement mergeAnalysesElem = parserParamsElem.Elements("HC").Elements("MergeAnalyses").FirstOrDefault();
XElement mergeMSAsElem = parserParamsElem.Elements("HC").Elements("MergeMSAs").FirstOrDefault();
XElement maxRootsElem = parserParamsElem.Elements("HC").Elements("MaxRoots").FirstOrDefault();
XElement maxAlternativesElem = parserParamsElem.Elements("HC").Elements("MaxAlternatives").FirstOrDefault();
if (delReappsElem != null)
delReapps = (int) delReappsElem;
if (guessRootsElem != null)
m_guessRoots = (bool) guessRootsElem;
if (mergeAnalysesElem != null)
m_mergeAnalyses = (bool) mergeAnalysesElem;
m_mergeAnalyses = (bool)mergeAnalysesElem;
if (mergeMSAsElem != null)
m_mergeMSAs = (bool)mergeMSAsElem;
if (maxRootsElem != null)
maxStemCount = int.Parse(maxRootsElem.Value);
if (maxAlternativesElem != null)
maxAlternatives = int.Parse(maxAlternativesElem.Value);
writer.WriteStartElement("LoadErrors");
if (m_mergeMSAs)
{
m_language = HCLoader.Load(m_cache, new XmlHCLoadErrorLogger(writer), out m_representativeMSAs);
}
else
Comment thread
jtmaxwell3 marked this conversation as resolved.
{
m_language = HCLoader.Load(m_cache, new XmlHCLoadErrorLogger(writer));
m_representativeMSAs = null;
}
writer.WriteEndElement();
}
m_morpher = new Morpher(m_traceManager, m_language) { DeletionReapplications = delReapps };
m_morpher.MaxStemCount = maxStemCount;
Expand All @@ -191,7 +206,7 @@ private XDocument ParseToXml(string form, bool tracing, IEnumerable<int> selectT
{
if (selectTraceMorphs != null)
{
var selectTraceMorphsSet = new HashSet<int>(selectTraceMorphs);
HashSet<int> selectTraceMorphsSet = new HashSet<int>(selectTraceMorphs.Select(hvo => RepresentativeMSAHvo(hvo)).Distinct());
m_morpher.LexEntrySelector = entry => selectTraceMorphsSet.Contains((int) entry.Properties[MsaID]);
m_morpher.RuleSelector = rule =>
{
Expand Down Expand Up @@ -233,6 +248,24 @@ private XDocument ParseToXml(string form, bool tracing, IEnumerable<int> selectT
return doc;
}

private int RepresentativeMSAHvo(int hvo)
{
if (m_representativeMSAs == null)
{
return hvo;
}
IMoMorphSynAnalysisRepository repository = m_cache.LangProject.Services.GetInstance<IMoMorphSynAnalysisRepository>();
if (repository.TryGetObject(hvo, out IMoMorphSynAnalysis msa) && m_representativeMSAs.ContainsKey(msa))
{
return m_representativeMSAs[msa].Hvo;
}
else
{
return hvo;
}

}

/// <summary>
/// Check integrity of phoneme-based natural classes (PhNCSegments)
/// when there are phonological features
Expand Down
19 changes: 18 additions & 1 deletion Src/LexText/ParserCore/ParseResult.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,12 +72,19 @@ public override int GetHashCode()
public class ParseAnalysis : IEquatable<ParseAnalysis>
{
private readonly ReadOnlyCollection<ParseMorph> m_morphs;
private readonly Dictionary<IMoMorphSynAnalysis, IMoMorphSynAnalysis> m_representativeMSAs;

public ParseAnalysis(IEnumerable<ParseMorph> morphs)
{
m_morphs = new ReadOnlyCollection<ParseMorph>(morphs.ToArray());
}

public ParseAnalysis(IEnumerable<ParseMorph> morphs, Dictionary<IMoMorphSynAnalysis, IMoMorphSynAnalysis> representativeMSAs)
{
m_morphs = new ReadOnlyCollection<ParseMorph>(morphs.ToArray());
m_representativeMSAs = representativeMSAs;
}

public ReadOnlyCollection<ParseMorph> Morphs
{
get { return m_morphs; }
Expand Down Expand Up @@ -115,7 +122,8 @@ public bool MatchesIWfiAnalysis(IWfiAnalysis analysis)
foreach (IWfiMorphBundle mb in analysis.MorphBundlesOS)
{
var current = this.Morphs[i++];
if (mb.MorphRA == current.Form && mb.MsaRA == current.Msa && mb.InflTypeRA == current.InflType &&
var mbMSA = RepresentativeMSA(mb.MsaRA);
if (mb.MorphRA == current.Form && mbMSA == current.Msa && mb.InflTypeRA == current.InflType &&
(current.GuessedString == null || EquivalentFormString(mb.Form, current.GuessedString)))
{
// Possibly matches condition (2), above.
Expand All @@ -132,6 +140,15 @@ public bool MatchesIWfiAnalysis(IWfiAnalysis analysis)
return false;
}

private IMoMorphSynAnalysis RepresentativeMSA(IMoMorphSynAnalysis msa)
{
if (msa != null && m_representativeMSAs != null && m_representativeMSAs.TryGetValue(msa, out IMoMorphSynAnalysis repMSA))
{
return repMSA;
}
return msa;
}

private bool EquivalentFormString(IMultiString multiString, string formString)
{
foreach (int ws in multiString.AvailableWritingSystemIds)
Expand Down
36 changes: 36 additions & 0 deletions Src/LexText/ParserCore/ParserCoreTests/HCLoaderTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -662,6 +662,42 @@ public void UnclassifedAffix()
Assert.That(rule.IsPartial, Is.True);
}

[Test]
public void RepresentativeMSAs()

@johnml1135 johnml1135 Sep 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This covers affix merging. Proposed additional cases:

Stem MSAs merge

  • Given an entry with two senses whose stem MSAs are EqualsMsa-equal
  • When the lexicon is loaded with merging on
  • Then there is one HC lexical entry, with the first sense's gloss

Survivor is a referenced MSA (the Aweti kãᴷ shape)

  • Given stem MSA A that no sense uses and an equal MSA B used by a sense
  • When loaded
  • Then the one entry carries B's hvo and B's sense gloss (not ???)

Variants

  • Given a variant of a main entry that has two equal stem MSAs
  • When loaded
  • Then one variant entry is created

Different MSAs stay separate

  • Given two stem MSAs with different POS in one entry, and equal MSAs in two different entries
  • When loaded
  • Then all of them load as separate entries

Ad-hoc prohibition on a merged-away MSA

  • Given a morpheme prohibition whose FirstMorphemeRA (and, separately, one of whose RestOfMorphsRS) is a merged-away MSA
  • When loaded
  • Then the co-occurrence rule is still attached to the kept morpheme

Saved analysis on a merged-away MSA

  • Given a WfiAnalysis whose morph bundle uses merged-away MSA B
  • When the parser returns the same parse with representative MSA A
  • Then MatchesIWfiAnalysis is true and ParseFiler neither adds a duplicate analysis nor disapproves the saved one

Option off

  • Given MergeMSAs = false
  • When loaded
  • Then duplicate MSAs load separately and matching is by exact MSA

@jtmaxwell3 jtmaxwell3 Sep 30, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I added "Option off". I think that the unit test has coverage that is comparable to other features in HCLoaderTests, but you can add more tests if you want.

@johnml1135 johnml1135 Oct 1, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the option-off test. The one case I'd still like covered is the main path: two senses on EqualsMsa-equal stem MSAs load as one HC entry. Happy for that to be a follow-up instead of blocking this PR. Want me to file an LT issue for it?

{
ILexEntry lexEntry = AddEntry(MoMorphTypeTags.kguidMorphSuffix, "ɯd", "gloss", new SandboxGenericMSA { MsaType = MsaType.kUnclassified, MainPOS = m_verb });
var senseFactory = Cache.ServiceLocator.GetInstance<ILexSenseFactory>();
var lexSense = senseFactory.Create();
lexEntry.SensesOS.Add(lexSense);
IMoUnclassifiedAffixMsa msa = Cache.ServiceLocator.GetInstance<IMoUnclassifiedAffixMsaFactory>().Create();
lexEntry.MorphoSyntaxAnalysesOC.Add(msa);
lexSense.MorphoSyntaxAnalysisRA = msa;
msa.PartOfSpeechRA = m_verb;

ILexEntry stemEntry = AddEntry(MoMorphTypeTags.kguidMorphStem, "a", "gloss", new SandboxGenericMSA { MsaType = MsaType.kStem });
var stemSense = senseFactory.Create();
stemEntry.SensesOS.Add(stemSense);
IMoStemMsa stemMsa = Cache.ServiceLocator.GetInstance<IMoStemMsaFactory>().Create();
stemEntry.MorphoSyntaxAnalysesOC.Add(stemMsa);
stemSense.MorphoSyntaxAnalysisRA = stemMsa;

m_loadErrors.Clear();
m_lang = HCLoader.Load(Cache, new TestHCLoadErrorLogger(m_loadErrors), out _);

Assert.That(m_lang.Strata[0].Entries.Count, Is.EqualTo(1));
Assert.That(m_lang.Strata[0].MorphologicalRules.Count, Is.EqualTo(1));
var rule = (AffixProcessRule)m_lang.Strata[0].MorphologicalRules[0];

Assert.That(rule.RequiredSyntacticFeatureStruct.ToString(), Is.EqualTo("[POS:V]"));
Assert.That(rule.Gloss, Is.EqualTo("gloss"));
Assert.That(rule.IsPartial, Is.True);

m_loadErrors.Clear();
m_lang = HCLoader.Load(Cache, new TestHCLoadErrorLogger(m_loadErrors));
Assert.That(m_lang.Strata[0].Entries.Count, Is.EqualTo(2));
Assert.That(m_lang.Strata[0].MorphologicalRules.Count, Is.EqualTo(2));
}

[Test]
public void AffixNoMorphTypeSet()
{
Expand Down
9 changes: 7 additions & 2 deletions Src/LexText/ParserUI/ParserParametersDlg.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ public class ParserParametersDlg : ParserParametersBase
private const string AcceptUnspecifiedGraphemes = "AcceptUnspecifiedGraphemes";
private const string GuessRoots = "GuessRoots";
private const string MergeAnalyses = "MergeAnalyses";
private const string MergeMSAs = "MergeMSAs";
private const string Strata = "Strata";

private const string XAmple = "XAmple";
Expand Down Expand Up @@ -282,8 +283,9 @@ public void SetDlgInfo(string title, string parserParameters, ILcmOwningSequence
m_dataGrid2.TableStyles[0].GridColumnStyles[2].Width = 130;
m_dataGrid2.TableStyles[0].GridColumnStyles[4].Width = 160;
m_dataGrid2.TableStyles[0].GridColumnStyles[6].Width = 90;
m_dataGrid2.TableStyles[0].GridColumnStyles[7].Width = 100;
m_dataGrid2.TableStyles[0].GridColumnStyles[8].Width = 400;
m_dataGrid2.TableStyles[0].GridColumnStyles[7].Width = 80;
m_dataGrid2.TableStyles[0].GridColumnStyles[8].Width = 100;
m_dataGrid2.TableStyles[0].GridColumnStyles[9].Width = 400;

m_compoundRules = compoundRules;
if (m_compoundRules?.Count > 0)
Expand Down Expand Up @@ -318,6 +320,8 @@ private void LoadParserData(DataSet dsParserParameters)
hcElem.Add(new XElement(GuessRoots, true));
if (hcElem.Element(MergeAnalyses) == null)
hcElem.Add(new XElement(MergeAnalyses, true));
if (hcElem.Element(MergeMSAs) == null)
hcElem.Add(new XElement(MergeMSAs, true));
if (hcElem.Element(Strata) == null)
hcElem.Add(new XElement(Strata, ""));

Expand Down Expand Up @@ -372,6 +376,7 @@ private DataTable CreateHCDataTable()
tblHC.Columns.Add(AcceptUnspecifiedGraphemes, typeof(bool));
tblHC.Columns.Add(GuessRoots, typeof(bool));
tblHC.Columns.Add(MergeAnalyses, typeof(bool));
tblHC.Columns.Add(MergeMSAs, typeof(bool));
tblHC.Columns.Add(MaxAlternatives, typeof(int));
tblHC.Columns.Add(Strata, typeof(string));
return tblHC;
Expand Down
Loading