algos + tests + docu #138
algos + tests + docu #138
Conversation
| ext-B (remove nil? (for [concept-B (concepts B)] (if (and (not-empty (first concept-B)) (not-empty (second concept-B))) (first concept-B))))] | ||
| (* (/ 1 (count ext-A)) (clojure.core.reducers/reduce + (for [ext-a ext-A] (apply max (helper-eval ext-a ext-B))))))) | ||
|
|
||
| (defn jaccard-index |
There was a problem hiding this comment.
There is already a Jaccard-Index within metrics.clj . Either use this one or relocate both into math/utils.clj
| [A B] | ||
| (difference (incidence-relation (normalize-context A)) (incidence-relation (normalize-context B)))) | ||
|
|
||
| (defn concept-ratio |
There was a problem hiding this comment.
This name and defintion is weired, since the function does not know anything about concepts or contexts. Inline (double ... ) and put a fitting comment there
There was a problem hiding this comment.
This was once used to determine the ratio of two different sets but was not actually used. Must've slipped when I cleaned the code
| @@ -0,0 +1,927 @@ | |||
| (ns conexp.fca.factorization | |||
| (is (= water-tiling (context water-tiling-test)))) | ||
|
|
||
| (deftest test-topfiberm | ||
| (is (= water-test-topfiberm (context water-test-topfiberm-test)))) |
| [a] | ||
| (into [] (apply map vector a))) | ||
|
|
||
| (defn make-matrix-from-context |
There was a problem hiding this comment.
Why have a string to 1/0 conversion here? Seems like a function from conexp.io.contexts should have been used
There was a problem hiding this comment.
Haven't found any context -> 1/0-matrix conversion
| (make-context-from-matrix | ||
| (first (context-size context)) | ||
| (second (context-size context)) | ||
| (into [] (flatten (make-matrix-from-context (context-to-string context)))))) |
There was a problem hiding this comment.
The dependence on context-to-string to create an incidence matrix may case side effects. Maybe a representation as used in the fast namespace is better.
| ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; | ||
| ;; helper functions | ||
|
|
||
| (defn make-matrix-from-concept |
There was a problem hiding this comment.
Cant this be done with the map function?
There was a problem hiding this comment.
This is the best I came up with :D
| (defn normalize-context | ||
| [context] | ||
| (make-context-from-matrix | ||
| (first (context-size context)) |
There was a problem hiding this comment.
count objects
count attributes
| ;; Unite-Operator for 2 matrices | ||
| (defn- unite | ||
| [A B] | ||
| (loop [i 0 C []] |
There was a problem hiding this comment.
Yep, this does the same as sum, tailored for the use-case here
|
|
||
| (defn convert-context-to-matrix | ||
| [context] | ||
| (into [] (flatten (make-matrix-from-context (context-to-string (normalize-context context)))))) |
There was a problem hiding this comment.
The dependence on context-to-string to create an incidence matrix may case side effects. Maybe a representation as used in the fast namespace is better.
There was a problem hiding this comment.
This PR need some refactoring.
- Remove undesirable dependencies
- Refactor loop/recurs to some, map etc
- Add speaking variable names
- Add Doc Strings to function on what inputs are expected, what the outputs are and links to the papers where the algorithms were introduced
| Every algorithm except asso and topfiberm need a context and a number k to produce factorizations. | ||
|
|
||
| ```clj | ||
| (:context (apply ->factorization-record (topfiberm water-context k tp SR)));; tp is a threshold from 0 to 1, Sr is a search radius |
There was a problem hiding this comment.
The factorization record should be the default return type for all factorization methods.
|
One of the tests fails |
No description provided.