For review: Custon numeric types

243 views
Skip to first unread message

Ralph Ritoch

unread,
Dec 9, 2014, 2:26:00 AM12/9/14
to cloju...@googlegroups.com
Hello,

   I have created a way to create custom numeric types with type specific operations in clojure. The code is available on github @ https://github.com/rritoch/clojure .  What this new code does is it checks if the number interfaces new clojure.lang.INumber interface, and if it does it uses the methods of that interface to compute the results of numeric operations.

There are 3 commits which produce this feature.

https://github.com/rritoch/clojure/commit/da081acfadcf427e4ef38db8c681e127aa9ed344

https://github.com/rritoch/clojure/commit/fd926946151e28e202587c24f5f1ebac5597a860

https://github.com/rritoch/clojure/commit/b19b071ad6ba2ef069862f46f0cd90c0c86572b0


I would like to contribute this code to be added to clojure as this opens up unlimited custom numeric operations, including the possibility of producing GPU and/or coprocessor accelerated vector operations.

Best Regards,
  Ralph Ritoch

Ralph Ritoch

unread,
Dec 9, 2014, 2:44:00 AM12/9/14
to cloju...@googlegroups.com
I apologize for the typo in the subject. It should read "For Review: Custom Numeric Types".

Reid McKenzie

unread,
Dec 9, 2014, 4:15:12 AM12/9/14
to cloju...@googlegroups.com
Just some quick thoughts. It's well past my bedtime so I'm sorry if this
comes off as curt or snapping, it's not intended as such. I think this
is an interesting idea and a problem that a couple of contrib libraries
have had to hack around and/or reinvent.

Please squash all three patches for review. It's easier to read one diff
than three put together mentally. You would almost certainly get asked
to so do if these patches were on JIRA, and I did a squash myself to
read these changes. https://github.com/arrdem/clojure/commit/37958a5

The gitignore changes almost certainly won't be accepted.

I note garbage whitespace in INumber.java, whitespace changes in
Numbers.java. The Core team doesn't have an official whitespace policy,
but it gets my goat at least.

How does this compare to the existing implementation? I slapped the
following naive benchmark together which suggests that these patches
generate a (small) slowdown compared to 1.6.0. 100,019ms compared to
99,346ms on my not especially beefy laptop. 1.7.0 top of tree clocks in
at 98,353ms. So a net slowdown. I would be surprised if the Core team
will take these changes at a slowdown. I may be wrong since it's small
and brings a win for library authors, but breaking even or coming out
ahead would probably help these changes along. Benchmarking with
primitive math?

<code>
(time
(let [ctors [int long float double bigint],
;; FIXME: primitive ops?
ops [+ +' - -' * *' /],
rng (range 1 1000)]
;; FIXME: use futures returning ((ctor, ctor, op), time-msg)
(doseq [lhs ctors, rhs ctors,
lval rng, rval rng,
op ops]
(let [l (lhs lval)
r (rhs rval)]
(op l r)))))
</code>

Can this completely replace say algo.generic? I think it does, at least
for the clojure.algo.generic.arithmetic namespace. It'd be interesting
to see interval arithmetic, arithmetic with error terms, composite
numbers, vectors or some other numeric type implemented as a proof of
concept.

It would also be interesting to here Mikera sound off on the
implications for numeric-tower, algo.generic and core.matrix all of
which AFAIK tackle the same problem (extending core's numbers).

Documentation? I know that there's not a whole lot officially "public"
in clojure.lang, but if this is a candidate for extension with custom
numeric types then it should probably have docs to assist therewith
rather than expecting users to reverse engineer clojure.core/+ or some
other function.

Will be interested to see where this thread goes,

Reid

Ralph Ritoch

unread,
Dec 9, 2014, 5:24:02 AM12/9/14
to cloju...@googlegroups.com
Reid McKenzie,

  Thanks for the feedback. You mentioned some fairly trivial issues which have nothing to do with this feature, such as whitespace and the gitignore additions which I added for personal use. If this feature is going to be adopted I can isolate this feature to a separate branch in a single commit.

Taking a look at the speed issues which is highly relevant the primary cost is with the minus function. To be minimally invasive I implemented it with conditional logic, a more invasive method of actually implementing the subtraction functions for all "Ops" and using it in every case would be faster.

https://github.com/rritoch/clojure/blob/master/src/jvm/clojure/lang/Numbers.java#L143-L149

This code hasn't been optimized, but I believe any speed losses can be eliminated by fully implementing subtraction.

The second slowdown in this code is extremely trivial, but worthy of note.

https://github.com/rritoch/clojure/blob/master/src/jvm/clojure/lang/Numbers.java#L1204-L1227

This code has an additional test which occurs before a fallback to Number types that don't implement INumber and it is completely unavoidable as far as I can tell.

If this feature get's accepted I have no problems with writing some documentation. I have been testing this with some custom unsigned types which could be used as a reference.

When it comes to speed optimization of numeric operations in general that is unrelated to this feature and should be handled separately. I don't see the reasoning behind the calls to combine, and opsWith, as I believe it would be much faster to skip these operations.  But I will work on implementing the subtraction across the system as that should eliminate speed losses from typical usage, including the tests you presented.

Best Regards,
  Ralph Ritoch

Ralph Ritoch

unread,
Dec 9, 2014, 5:29:18 AM12/9/14
to cloju...@googlegroups.com
Read McKenzie,

This fork also has another feature in it of namespace isolation which may be another cause for some speed loss. If you downloaded the changes to Namespace and the additional NamespaceContainer, that is possibly a contributing factor to the benchmark changes.

Best Regards,
  Ralph Ritoch

Ralph Ritoch

unread,
Dec 9, 2014, 8:16:53 AM12/9/14
to cloju...@googlegroups.com
Reid McKenzie,

   I implemented the subtraction functions to remove the conditional logic and the performance is now comparable to version 1.6.0.

Commit: https://github.com/rritoch/clojure/commit/b2c87ca4da044a1511c193b13233416513a953dc

I ran your tests and am getting speed variations between 102,702ms and 97,632ms, but most of the times I run it now it is coming in under 98,000ms.  When I run the tests against version 1.6.0 I am getting variations between 102,340ms and 96,899ms. There may be some impact on speed but it is within the variations of the test itself, at least on my machine.

Best Regards,
  Ralph Ritoch

Alex Miller

unread,
Dec 9, 2014, 11:27:24 AM12/9/14
to cloju...@googlegroups.com
Hey Ralph,

I work at Cognitect on Clojure. Thanks for exploring these ideas and proposing a patch (and thanks to Reid for making many of the same initial suggestions I had).

In general, when we consider changes for Clojure, we try to approach from the perspective of what problem we are trying to solve. In this case, I believe from what I've seen on irc that the problem you're trying to solve here is opening up a path for unsigned math (I think?).

The question then becomes, what are the possible approaches towards that goal? Certainly the path you've gone down is one. There may perhaps be others. They will have various tradeoffs in maintenance, performance, consistency with the rest of Clojure, etc. Generally we prefer to work from the problem first rather than by leading with patches.

Generally the place we have written down these kind of things is by making a page on the design wiki (http://dev.clojure.org/display/design/Home) - you should have rights to do that. In addition to the original goal, there are also considerations about performance (likely to require a fairly broad spectrum of tests for such an important area) and other Clojure platforms (ClojureScript and ClojureCLR). 

None of this is to say this is a bad idea or anything - I think it's kind of neat that so few changes were actually necessary to open this up. However, my general impression is that this is not at the top of the list for what the core team is likely to focus on in the near future.

Alex 
(I'm puredanger on #clojure if you ever want to catch me there)




--
You received this message because you are subscribed to the Google Groups "Clojure Dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to clojure-dev...@googlegroups.com.
To post to this group, send email to cloju...@googlegroups.com.
Visit this group at http://groups.google.com/group/clojure-dev.
For more options, visit https://groups.google.com/d/optout.

Ralph Ritoch

unread,
Dec 9, 2014, 10:02:24 PM12/9/14
to cloju...@googlegroups.com
Alex Miller,

   Thank you for your response but I am a professional programmer and not commissioned for this project. Other than "donating" this code, I don't currently have the time to put additional effort into this project as I already have the features I need. This implementation, to my knowledge, is the best way, based on my 30 years of programming experience. C++, Pike, Ruby, and many other languages allow creation of custom numeric types simply by adding specific functions. Java doesn't make things that easy. With java the only way to implement this in the standard way would be to use reflection which is inefficient, making interfaces the best option. This feature was made for a personal project and the tone at irc #clojure implies that while this is a feature developers want, clojure has no interest in adding support for additional numeric types and that it was a big conflict simply to get support for the primitive types.  I have signed the contributor agreement so you are free to do what you want with this code, but at the moment I do not have the time available to pursue this feature.

Best Regards,
  Ralph Ritoch

Alex Miller

unread,
Dec 10, 2014, 10:06:20 AM12/10/14
to cloju...@googlegroups.com
On Tue, Dec 9, 2014 at 9:02 PM, Ralph Ritoch <rri...@gmail.com> wrote:
Alex Miller,

   Thank you for your response but I am a professional programmer and not commissioned for this project. Other than "donating" this code, I don't currently have the time to put additional effort into this project as I already have the features I need.

Understood. If there is community support, perhaps someone else will be willing to make an investment.
 
This implementation, to my knowledge, is the best way, based on my 30 years of programming experience. C++, Pike, Ruby, and many other languages allow creation of custom numeric types simply by adding specific functions. Java doesn't make things that easy. With java the only way to implement this in the standard way would be to use reflection which is inefficient, making interfaces the best option. This feature was made for a personal project and the tone at irc #clojure implies that while this is a feature developers want, clojure has no interest in adding support for additional numeric types and that it was a big conflict simply to get support for the primitive types. 

As far as I know, you haven't interacted with me or others that influence those decisions other than here, so not sure what "tone" was transmitted. I don't recall there being "conflict" to get support for primitive types (although that's a bit before I was involved with Clojure's dev). I do remember a number of approaches being considered wrt autopromotion, contagion, etc. In general, the idea of programming to abstractions (interfaces, protocols) is strongly preferred, so I think in that vein your proposal is harmonious with the current Clojure approach. Performance is the main concern competing with abstraction here and I think your patch so far does a pretty good job of mitigating that.
 
I have signed the contributor agreement so you are free to do what you want with this code, but at the moment I do not have the time available to pursue this feature.

Thanks.
Reply all
Reply to author
Forward
0 new messages