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

[TTFS] clean interior-point kernels to improve inference #228

Merged
merged 4 commits into from
Oct 7, 2022

Conversation

frapac
Copy link
Collaborator

@frapac frapac commented Oct 2, 2022

  • rewrite one-line functions to improve readibility
  • use @inbounds @simd in a systematic way before for loops
  • fix easy broadcast calls in kernels.jl
  • rewrite broadcast operators in print_init to ease compiler job (1s gain in itself)

x-ref #186

Currently, we get on MadNLP#master:

InferenceTimingNode: 5.120791/8.009915 on Core.Compiler.Timings.ROOT() with 9 direct children

With this PR:

InferenceTimingNode: 4.037863/6.572491 on Core.Compiler.Timings.ROOT() with 9 direct children 

(note that this is independent from the results already reported in #227 )

As a reminder, when we get InferenceTimingNode: x/y, that means we are spending x seconds in the LLVM compilation, and y seconds in total (compilation + precompilation). With this PR, it takes 4s to compile the call to MadNLP, 2.5s to precompile the code and do the type inference.

- use @inbounds @simd in a systematic way before for loops
- fix easy broadcast calls in kernels.jl
- rewrite broadcast operators in print_init to ease compiler job
@codecov-commenter
Copy link

codecov-commenter commented Oct 3, 2022

Codecov Report

Merging #228 (87a10c9) into master (6ac071a) will decrease coverage by 0.09%.
The diff coverage is 94.88%.

@@            Coverage Diff             @@
##           master     #228      +/-   ##
==========================================
- Coverage   74.29%   74.19%   -0.10%     
==========================================
  Files          37       38       +1     
  Lines        3684     3697      +13     
==========================================
+ Hits         2737     2743       +6     
- Misses        947      954       +7     
Impacted Files Coverage Δ
src/LinearSolvers/linearsolvers.jl 20.00% <ø> (ø)
src/MadNLP.jl 0.00% <ø> (-33.34%) ⬇️
src/IPM/kernels.jl 84.16% <93.48%> (+0.57%) ⬆️
src/IPM/solver.jl 91.39% <96.00%> (ø)
src/IPM/utils.jl 93.47% <98.07%> (+1.95%) ⬆️
src/IPM/callbacks.jl 100.00% <100.00%> (ø)
src/IPM/factorization.jl 100.00% <100.00%> (ø)
src/IPM/restoration.jl 100.00% <100.00%> (ø)
src/nlpmodels.jl 96.77% <100.00%> (ø)
src/utils.jl 85.71% <100.00%> (+0.71%) ⬆️
... and 3 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Member

@sshin23 sshin23 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good @frapac! only have a minor comment

src/IPM/utils.jl Show resolved Hide resolved
@frapac frapac merged commit 8260ac9 into master Oct 7, 2022
@frapac frapac deleted the fp/clean_kernels branch October 7, 2022 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants