htm-community / comportex

Hierarchical Temporal Memory in Clojure
154 stars 27 forks source link

REPL improvement: Implement toString for RegionNetwork #2

Closed mrcslws closed 9 years ago

mrcslws commented 9 years ago

When a RegionNetwork's value is returned to the REPL, I'm immediately filled with regret :)

On the REPL wiki page, I dodged this by immediately defing it.

(def model-t1 (htm-step model))

But it would be nice if I could just do (htm-step model) and get quick, comprehensible output. Some sort of summary. That'd make it easier to explore the API.

(I haven't investigated whether the toString approaches for Clojure and ClojureScript are compatible...)

floybix commented 9 years ago

I think a problem with this currently is that it always truncates even if you don't want it to. Serialisation in clojure is just printing so i think we need to cater for printing the full data structure.

Maybe we could have a new var like core/*comportex-print-length* which can be (set!) in an interactive session to 3?

floybix commented 9 years ago

Or just do the truncation on pprint, not print

mrcslws commented 9 years ago

I'll dive in and make sense of this. Thinking in first principles, I'd be really surprised if serialization consumes *print-length*. That would be 100% wrong, in a way that Clojure is never wrong.

Regardless, I think you're right that hardcoding it to 3 is bad. I'll at least make it customizable.

mrcslws commented 9 years ago

This space isn't well-documented unless you know what to look for: https://clojuredocs.org/clojure.core/*print-dup*

(binding [*print-length* 2]
    (prn {:a "a" :b "b" :c "c"}))
; {:c "c", :b "b", ...}
;= nil
(binding [*print-length* 2 *print-dup* true]
    (prn {:a "a" :b "b" :c "c"}))
; #=(clojure.lang.PersistentArrayMap/create {:c "c", :b "b", :a "a"})
;= nil

Note that it requires you to implement the print-dup multimethod for non-built-in types. So, for example:

(binding [*print-length* 3 *print-dup* true] (prn (reify)))
; IllegalArgumentException No method in multimethod 'print-dup' for dispatch value: class org.nfrac.comportex.demos.directional_steps_1d$eval19204$reify__19205  clojure.lang.MultiFn.getFn (MultiFn.java:160)

so we hit the same exception for Comportex's types currently.

floybix commented 9 years ago

Ooh, I didn't know that. You were right about Clojure of course.

mrcslws commented 9 years ago

...oh, but print-dup is already defined for records. So it's just the reifys (e.g. in encoders.cljx) that throw it off, currently.

floybix commented 9 years ago

Yah, of course you can't serialize a closure. Should make em types I guess. Not that I have an immediate need for serialization.

mrcslws commented 9 years ago

Regarding *comportex-print-length* (which I think is worthwhile), it's hard to put it in core currently because of the resulting circular dependency between util and core.

Options I can think of: a. Move the print-method-truncate functions to core b. Define a constant in util c. Create a config.cljx which contains this kind of thing

floybix commented 9 years ago

we could put them in core; i'd be inclined to move the (print-method-truncate) calls out of cells.cljx and synapses.cljx there too so it's all together. or maybe a new comportex.repl namespace but probably don't want to have to load another one.

floybix commented 9 years ago

tbqh I have really no experience in serialization. How would you serialize an encoder given then that it has an arbitrary selector (pre-transform) function wrapping it?

floybix commented 9 years ago

it is dawning on me that that selector (pre-transform) should not be attached to an encoder, or at least should not be part of an HTM model, but rather some separate property of a sensor as it interacts with a world.

mrcslws commented 9 years ago

If you've got something in mind, I don't want to interrupt. But another option is to lazy-implement serialization... as in, wait until someone wishes we had it, then build it. When there's a clearer idea of what to design around.

Then again, it might be a healthy exercise.