Wow. The first version of this PR used Cython memoryviews, which added a whopping 14 kLOC of generated C to the repo. Switched back to bare pointers to keep the compile times within bounds. It would be interesting to hear the Cython team's point of view on that. @sturlamolden @robertwb @markflorisson88 @dagss
The priority has always been to produce the most optimized runtime code, with compile time being way down on the list of priorities, if even thought about at all. This means that there's likely a lot of low-hanging fruit for those who find the compile time too slow (do you have some specific stats?). Also, while Cython generates very verbose C code, much of it is optimized away. While the lines-of-code and compile times are related, they should not be conflated. The recently added common_utility_include_dir option to cythonize(...) might help as well--it allows utility code to be shared across multiple generated files. Perhaps a faster -c dbg option for development would be handy. - Robert
Den 10. sep. 2013 kl. 02:14 skrev Robert Bradshaw <robertwb@gmail.com>:
Wow. The first version of this PR used Cython memoryviews, which added a whopping 14 kLOC of generated C to the repo. Switched back to bare pointers to keep the compile times within bounds. It would be interesting to hear the Cython team's point of view on that. @sturlamolden @robertwb @markflorisson88 @dagss
The priority has always been to produce the most optimized runtime code, with compile time being way down on the list of priorities, if even thought about at all.
I thought perhaps the problem was the number of pyx files they had to compile. I.e. that the utility code is included in multiple C files, since they complained about 14 kLOC extra. But to that they answered: " larsmans commented a day ago The problem with typed memoryviews is simply that they're different from what we were doing. We'll have to change a lot of code and habits, being careful not to lose performance due to memoryview overhead." So I am not sure what the problem really was. Bloat or just an unfamiliar API? There was also a strange comment about tail-call optimization being prevented in cdef functions that are not declared nogil – due to the refnanny. I am not sure what to make of that. Sturla
Robert Bradshaw, 10.09.2013 02:14:
Wow. The first version of this PR used Cython memoryviews, which added a whopping 14 kLOC of generated C to the repo. Switched back to bare pointers to keep the compile times within bounds. It would be interesting to hear the Cython team's point of view on that. @sturlamolden @robertwb @markflorisson88 @dagss
Is there a link to this discussion? Stefan
On Sep 12, 2013, at 6:33 PM, Stefan Behnel <stefan_ml@behnel.de> wrote:
Robert Bradshaw, 10.09.2013 02:14:
Wow. The first version of this PR used Cython memoryviews, which added a whopping 14 kLOC of generated C to the repo. Switched back to bare pointers to keep the compile times within bounds. It would be interesting to hear the Cython team's point of view on that. @sturlamolden @robertwb @markflorisson88 @dagss
Is there a link to this discussion?
https://github.com/scikit-learn/scikit-learn/pull/2426 Sturla
participants (3)
-
Robert Bradshaw -
Stefan Behnel -
Sturla Molden