Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

julia 0.6 broadcast integration #327

Merged
merged 13 commits into from
Oct 24, 2017
Merged

Conversation

iblislin
Copy link
Collaborator

@iblislin iblislin commented Aug 14, 2017

This work is derived from #321 (comment)

This PR make generic function do broadcast call correctly.

e.g.

julia> using MarketData

julia> f(a, b, c, d, e) = a * b + c - d + e 

julia> f.(1, 1, cl, ohlc, op[1:10])  # this was borken before this PR
10x4 TimeSeries.TimeArray{Float64,2,Date,Array{Float64,2}} 2000-01-03 to 2000-01-14 

             Close_Open_Open  Close_High_Open  Close_Low_Open  Close_Close_Open 
2000-01-03 | 112.94           105.32           116.13          105.88     
2000-01-04 | 103.5            101.13           110.56          109.25
2000-01-05 | 105.0            98.19            105.75          104.75      
2000-01-06 | 96.0             95.12            107.12          107.12

2000-01-11 | 93.75            90.31            99.19           96.94
2000-01-12 | 88.19            87.69            96.69           96.0
2000-01-13 | 97.75            93.48            99.73           95.48  
2000-01-14 | 101.44           99.19            102.06          101.0

Performance

The performance of current implementation is quite poor and the memory requirement is larger :(.
(I'm going to read more about performance tips. Please advise)

A test case for log.(2, cl .+ 1) which the works fine before.
Before:

julia> @benchmark log.(2, $cl .+ 1) 
BenchmarkTools.Trial:               
  memory estimate:  38.28 KiB       
  allocs estimate:  1269            
  --------------                    
  minimum time:     111.731 μs (0.00% GC)                                
  median time:      114.278 μs (0.00% GC)                                
  mean time:        118.402 μs (2.94% GC)                                
  maximum time:     2.330 ms (91.95% GC)                                 
  --------------                    
  samples:          10000           
  evals/sample:     1

After:

julia> @benchmark log.(2, $cl .+ 1) 
BenchmarkTools.Trial:               
  memory estimate:  85.19 KiB       
  allocs estimate:  2554            
  --------------                    
  minimum time:     218.412 μs (0.00% GC)                                
  median time:      224.006 μs (0.00% GC)                                
  mean time:        238.519 μs (4.23% GC)                                
  maximum time:     3.578 ms (87.72% GC)                                 
  --------------                    
  samples:          10000           
  evals/sample:     1

TODO

  • add test cases
  • improve performance

@iblislin iblislin force-pushed the broadcast branch 2 times, most recently from 17aa9da to 3444ef5 Compare August 14, 2017 08:25
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) to 85.012% when pulling 3444ef5 on iblis17:broadcast into 3de29a7 on JuliaStats:master.

@iblislin
Copy link
Collaborator Author

iblislin commented Sep 3, 2017

current patch set:

julia> @benchmark log.(2, $cl .+ 1)                                                                                                               
BenchmarkTools.Trial:               
  memory estimate:  41.47 KiB       
  allocs estimate:  1251            
  --------------                    
  minimum time:     93.868 μs (0.00% GC)                                                                                                          
  median time:      96.204 μs (0.00% GC)                                 
  mean time:        100.758 μs (3.53% GC)                                                                                                         
  maximum time:     2.244 ms (92.37% GC)                                                                                                          
  --------------                    
  samples:          10000           
  evals/sample:     1               

shell> git log | head                                                                                                                             
commit da49c48a78f633d5e56c3659d38bde1132212926                                                                                                   
Author: Iblis Lin <iblis@hs.ntnu.edu.tw>                                                                                                          
Date:   Sun Sep 3 19:19:13 2017 +0800                                                                                                             
                                                                                                                                                  
    broadcast: wip: rewrite broadcast_c in generated func                                                                                         

@coveralls
Copy link

Coverage Status

Coverage increased (+1.1%) to 86.192% when pulling 74c66a8 on iblis17:broadcast into 3de29a7 on JuliaStats:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.3%) to 85.377% when pulling a1c412f on iblis17:broadcast into 3de29a7 on JuliaStats:master.

@iblislin iblislin changed the title WIP: julia 0.6 broadcast integration julia 0.6 broadcast integration Sep 18, 2017
@iblislin
Copy link
Collaborator Author

I think it's ready for review

@iblislin iblislin added this to the 0.11.0 milestone Sep 18, 2017
@coveralls
Copy link

Coverage Status

Coverage increased (+0.3%) to 85.377% when pulling 31ca453 on iblis17:broadcast into 3de29a7 on JuliaStats:master.

@iblislin
Copy link
Collaborator Author

Bump?

@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 86.093% when pulling 1acf2c6 on iblis17:broadcast into ac8b6cb on JuliaStats:master.

@iblislin
Copy link
Collaborator Author

I'm going to merge this PR tmr, if no objection

@iblislin iblislin merged commit 6ddf24e into JuliaStats:master Oct 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants