Skip to content

Commit

Permalink
assorted minor comment and doc stuff
Browse files Browse the repository at this point in the history
  • Loading branch information
sdcondon committed Sep 29, 2024
1 parent 2968f12 commit 8a982ae
Show file tree
Hide file tree
Showing 7 changed files with 21 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ public IEnumerable<TValue> GetSubsumed(CNFClause clause)
return ExpandNode(root, 0);

// NB: subsumed clauses will have equal or higher vector elements.
// We allow zero-valued elements to be omitted from the vectors (so that we don't have to know what identifiers are possible ahead of time).
// We allow zero-valued elements to be omitted from the vectors (so that we don't have to know what features are possible ahead of time).
// This makes the logic here a little similar to what you'd find in a set trie when querying for supersets.
IEnumerable<TValue> ExpandNode(IFeatureVectorIndexNode<TFeature, TValue> node, int vectorComponentIndex)
{
Expand Down
2 changes: 2 additions & 0 deletions src/SCFirstOrderLogic/SentenceFormatting/SentenceFormatter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ namespace SCFirstOrderLogic.SentenceFormatting;
// TODO-FEATURE: Will ultimately want something that is more intelligent with brackets (i.e. drops them where not needed).
// Will need precedence list in here - then presumably not too tough to include brackets or not based on the relative priority
// of current op and child.
// TODO-BREAKING-FEATURE: Allow for configuration of whether zero-arity functions (and predicates?) should have brackets or not.
// Or roll into labeller (perhaps renamed - or separate interface - or perhaps some broader refactoring to do here) so can be by identifier?
public class SentenceFormatter
{
private const char PrecedenceBracketL = '[';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ namespace SCFirstOrderLogic.SentenceManipulation.VariableManipulation;
/// </para>
/// <para>
/// NB: of course, such comparison is non-trivial in terms of performance. When an unambiguous ordering of
/// literals can be established, consider transformation via <see cref="VariableManipulationExtensions.Ordinalise(Term)"/>
/// followed by equality comparison using plain old <see cref="object.Equals(object?)"/> instead.
/// literals can be established, instead consider prior transformation via <see cref="VariableManipulationExtensions.Ordinalise(Term)"/>,
/// followed by equality comparison using plain old <see cref="object.Equals(object?)"/>.
/// </para>
/// </summary>
// TODO-PERFORMANCE: The doc above does make it clear that this is a last resort, but I should defo take some time to try
Expand All @@ -36,7 +36,7 @@ public bool Equals(CNFClause? x, CNFClause? y)
}
else
{
return TryUpdateUnifier(x, y, new(), new());
return TryUpdateVariableMap(x, y, new(), new());
}
}

Expand All @@ -59,7 +59,7 @@ public bool Equals(Literal? x, Literal? y)
}
else
{
return TryUpdateUnifier(x, y, new(), new());
return TryUpdateVariableMap(x, y, new(), new());
}
}

Expand All @@ -82,7 +82,7 @@ public bool Equals(Predicate? x, Predicate? y)
}
else
{
return TryUpdateUnifier(x, y, new(), new());
return TryUpdateVariableMap(x, y, new(), new());
}
}

Expand All @@ -105,7 +105,7 @@ public bool Equals(Term? x, Term? y)
}
else
{
return TryUpdateUnifier(x, y, new(), new());
return TryUpdateVariableMap(x, y, new(), new());
}
}

Expand All @@ -115,7 +115,7 @@ public int GetHashCode([DisallowNull] Term obj)
return TransformForHashCode(obj).GetHashCode();
}

private static bool TryUpdateUnifier(
private static bool TryUpdateVariableMap(
CNFClause x,
CNFClause y,
Dictionary<VariableReference, VariableReference> xToY,
Expand All @@ -128,7 +128,7 @@ private static bool TryUpdateUnifier(

foreach (var literals in x.Literals.Zip(y.Literals, (x, y) => (x, y)))
{
if (!TryUpdateUnifier(literals.x, literals.y, xToY, yToX))
if (!TryUpdateVariableMap(literals.x, literals.y, xToY, yToX))
{
return false;
}
Expand All @@ -137,7 +137,7 @@ private static bool TryUpdateUnifier(
return true;
}

private static bool TryUpdateUnifier(
private static bool TryUpdateVariableMap(
Literal x,
Literal y,
Dictionary<VariableReference, VariableReference> xToY,
Expand All @@ -148,10 +148,10 @@ private static bool TryUpdateUnifier(
return false;
}

return TryUpdateUnifier(x.Predicate, y.Predicate, xToY, yToX);
return TryUpdateVariableMap(x.Predicate, y.Predicate, xToY, yToX);
}

private static bool TryUpdateUnifier(
private static bool TryUpdateVariableMap(
Predicate x,
Predicate y,
Dictionary<VariableReference, VariableReference> xToY,
Expand All @@ -165,7 +165,7 @@ private static bool TryUpdateUnifier(

foreach (var args in x.Arguments.Zip(y.Arguments, (x, y) => (x, y)))
{
if (!TryUpdateUnifier(args.x, args.y, xToY, yToX))
if (!TryUpdateVariableMap(args.x, args.y, xToY, yToX))
{
return false;
}
Expand All @@ -174,21 +174,21 @@ private static bool TryUpdateUnifier(
return true;
}

private static bool TryUpdateUnifier(
private static bool TryUpdateVariableMap(
Term x,
Term y,
Dictionary<VariableReference, VariableReference> xToY,
Dictionary<VariableReference, VariableReference> yToX)
{
return (x, y) switch
{
(VariableReference variableX, VariableReference variableY) => TryUpdateUnifier(variableX, variableY, xToY, yToX),
(Function functionX, Function functionY) => TryUpdateUnifier(functionX, functionY, xToY, yToX),
(VariableReference variableX, VariableReference variableY) => TryUpdateVariableMap(variableX, variableY, xToY, yToX),
(Function functionX, Function functionY) => TryUpdateVariableMap(functionX, functionY, xToY, yToX),
_ => false
};
}

private static bool TryUpdateUnifier(
private static bool TryUpdateVariableMap(
VariableReference x,
VariableReference y,
Dictionary<VariableReference, VariableReference> xToY,
Expand All @@ -215,7 +215,7 @@ private static bool TryUpdateUnifier(
return true;
}

private static bool TryUpdateUnifier(
private static bool TryUpdateVariableMap(
Function x,
Function y,
Dictionary<VariableReference, VariableReference> xToY,
Expand All @@ -228,7 +228,7 @@ private static bool TryUpdateUnifier(

for (int i = 0; i < x.Arguments.Count; i++)
{
if (!TryUpdateUnifier(x.Arguments[i], y.Arguments[i], xToY, yToX))
if (!TryUpdateVariableMap(x.Arguments[i], y.Arguments[i], xToY, yToX))
{
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ namespace SCFirstOrderLogic.TermIndexing;
/// <summary>
/// An implementation of a discrimination tree for <see cref="Term"/>s - specifically, one for which the attached values are the terms themselves.
/// </summary>
/// <seealso href="https://www.google.com/search?q=discrimination+tree"/>
public class AsyncDiscriminationTree
{
private readonly AsyncDiscriminationTree<Term> actualTree;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ namespace SCFirstOrderLogic.TermIndexing;
/// An implementation of a discrimination tree for <see cref="Term"/>s.
/// </summary>
/// <typeparam name="TValue">The type of value attached for each term.</typeparam>
/// <seealso href="https://www.google.com/search?q=discrimination+tree"/>
// NB: not a TODO just yet, but - while it's not terrible - there are a few aspects of this
// class that aren't great from a performance perspective. Notably, while the recursive iterator
// approach used for the retrieval methods may be easy to understand, it will make a lot of heap
Expand Down
1 change: 0 additions & 1 deletion src/SCFirstOrderLogic/TermIndexing/DiscriminationTree.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ namespace SCFirstOrderLogic.TermIndexing;
/// Discrimination trees are particularly well-suited to (i.e. performant at) looking up generalisations of a query term.
/// </para>
/// </summary>
/// <seealso href="https://www.google.com/search?q=discrimination+tree"/>
public class DiscriminationTree
{
private readonly DiscriminationTree<Term> actualTree;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ namespace SCFirstOrderLogic.TermIndexing;
/// </para>
/// </summary>
/// <typeparam name="TValue">The type of value attached for each term.</typeparam>
/// <seealso href="https://www.google.com/search?q=discrimination+tree"/>
// NB: not a TODO just yet, but - while it's not terrible - there are a few aspects of this
// class that aren't great from a performance perspective. Notably, while the recursive iterator
// approach used for the retrieval methods may be easy to understand, it will make a lot of heap
Expand Down

0 comments on commit 8a982ae

Please sign in to comment.