Changeset 15806 for branches

Timestamp:

02/22/18 13:46:16 (7 years ago)

Author:

gkronber

Message:

#2886 made a few comments

Location:

branches/2886_SymRegGrammarEnumeration/HeuristicLab.Algorithms.DataAnalysis.SymRegGrammarEnumeration/GrammarEnumeration

Files:

• Grammar.cs (modified) (3 diffs)
• GrammarEnumerationAlgorithm.cs (modified) (2 diffs)
• SearchDataStructure.cs (modified) (3 diffs)

branches/2886_SymRegGrammarEnumeration/HeuristicLab.Algorithms.DataAnalysis.SymRegGrammarEnumeration/GrammarEnumeration/Grammar.cs

@@ -204 +204 @@
         for (int i = 0; i < isFactorRemaining.Length; i++) {
           if (!isFactorRemaining[i]) continue;
-          if (isFactorRemaining.Count(b => b) <= 2) break; // Until we have constants, we can't cancel out all terms.
+          if (isFactorRemaining.Count() <= 2) break; // Until we have constants, we can't cancel out all terms.
 
           var currFactor = childHashes[i];
@@ -241 +241 @@
       }
 
-      return $"[{hashesArray.Aggregate(operatorSym.StringRepresentation, (result, ti) => string.Concat(result, " ° ", ti))}]";
+      return $"[{hashesArray.Aggregate(operatorSym.StringRepresentation, (result, ti) => string.Concat(result, " ° ", ti))}]";      // TODO: use string join instead of string.Concat
     }
 
@@ -268 +268 @@
       Debug.Assert(sentence.All(s => s is TerminalSymbol), "Trying to evaluate symbol sequence with nonterminalsymbols!");
 
-      symbolicExpressionGrammar.ConfigureAsDefaultRegressionGrammar();
+      symbolicExpressionGrammar.ConfigureAsDefaultRegressionGrammar();     // TODO: not necessary to call this for each sentence
 
       var rootNode = rootSy.CreateTreeNode();

branches/2886_SymRegGrammarEnumeration/HeuristicLab.Algorithms.DataAnalysis.SymRegGrammarEnumeration/GrammarEnumeration/GrammarEnumerationAlgorithm.cs

@@ -136 +136 @@
           foreach (Production productionAlternative in expandedSymbol.Alternatives) {
             SymbolString newPhrase = new SymbolString(currPhrase);
-            newPhrase.RemoveAt(nonterminalSymbolIndex);
+            newPhrase.RemoveAt(nonterminalSymbolIndex);     // TODO: removeat and insertRange are both O(n)
             newPhrase.InsertRange(nonterminalSymbolIndex, productionAlternative);
 
@@ -181 +181 @@
         AllSentences[hash].Add(sentence);
       else
-        AllSentences[hash] = new List<SymbolString> { sentence };
+        AllSentences[hash] = new List<SymbolString> { sentence }; //TODO: here we store all sentences even if they have the same hash value, this is not strictly necessary
     }
 

branches/2886_SymRegGrammarEnumeration/HeuristicLab.Algorithms.DataAnalysis.SymRegGrammarEnumeration/GrammarEnumeration/SearchDataStructure.cs

@@ -5 +5 @@
 namespace HeuristicLab.Algorithms.DataAnalysis.SymRegGrammarEnumeration.GrammarEnumeration {
 
+  // TODO: struct not useful here because you are boxing the structs with a new call below
   public struct StoredSymbolString {
     public readonly int Hash;
@@ -66 +67 @@
         int indexOfHash = rand.Next(list.Count);
         int result = list[indexOfHash];
-        list.RemoveAt(indexOfHash);
+        list.RemoveAt(indexOfHash);  // TODO: beware this is O(n), at some point in time we should fix this
         return result;
       };
@@ -79 +80 @@
       SymbolString result = storedValues[hash];
       storedValues.Remove(hash);
-      return new StoredSymbolString(hash, result);
+      return new StoredSymbolString(hash, result);  // TODO: new for structs creates a box anyway
     }
 
     public void Store(int hash, SymbolString s) {
-      storeInternal.Invoke(hash);
+      storeInternal.Invoke(hash); // TODO: explicit .Invoke() not necessary, just storeInternal(hash) is ok
       storedValues[hash] = s;
     }