-
Notifications
You must be signed in to change notification settings - Fork 21
preallocated coordinate format vectors #473
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
base: main
Are you sure you want to change the base?
Conversation
|
@arnavk23 If we support "double-buffering", we should by default create empty buffers and resize them on demand if needed. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #473 +/- ##
===========================================
- Coverage 89.11% 73.48% -15.63%
===========================================
Files 5 7 +2
Lines 790 1716 +926
===========================================
+ Hits 704 1261 +557
- Misses 86 455 +369 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@amontoison Any changes needed here? |
|
@arnavk23 Don't abuse with the ping ;) |
Sorry for the same. I was just looking over these pull requests and had some more ideas to implement. |
|
@amontoison please review this pr. |
|
@arnavk23 The CI build for documentation is failing. |
|
I added a quick comment, I will review the PR in more detail at the end of the week. |
|
@arnavk23 I will review this PR tomorrow but after no review until Friday 31. |
What does the no review mean here? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @arnavk23 for the PR. Please do not ping us every other day, once you requested a review from Github it appears in our list. Reviewing PR is not our main occupation so it requires some time, thank you for your understanding 😀.
It means that I won’t be able to review any PRs in JSO for about a week. |
Not this week, maybe next Friday. |
|
@tmigot I have tried all the methods there are to pass the Documentation check. I think we have to revert back to api.md methodology. |
Oh sorry for that, I checked and strangely enough CUTEst is doing differently than the other JSO packages... So, yes, you have to add them. |
|
@tmigot I have reverted it back to earlier working documentation change using api.md |
|
Why do you need it in a different file just for three docstrings though ? references.md with the others is sufficient. |
|
@amontoison all checks pass and Tangi has reviewed this file completely. |
Successfully implemented vector preallocation optimization to eliminate memory allocations in coordinate format functions (
cons_coord,hess_coord) and type conversion operations.Changes Made
1. Enhanced CUTEstModel Struct (
src/model.jl)Added 8 new preallocated workspace vectors to the
CUTEstModelstruct:2. Updated Constructor Logic
nnzj,nnzh,ncon,nvar)3. Optimized Coordinate Format Functions (
src/julia_interface.jl)cons_coord! function:
jac_coord_rows,jac_coord_cols,jac_coord_valshess_coord! function:
hess_coord_valsworkspace4. Added Type Conversion Helpers
Performance Results
cons_coord!()hess_coord!()cons_coord()Test Results
Unconstrained Problem (ROSENBR):
hess_coord!: 0 allocations, 384nsConstrained Problem (HS6):
cons_coord!: 0 allocations, 301nshess_coord!: 0 allocations, 394nsImplementation Notes
Design Decisions
Technical Challenges Solved
.!linearsyntax for element-wise negationFiles Modified
src/model.jl: Enhanced CUTEstModel struct and constructorsrc/julia_interface.jl: Updated coordinate functions and added helperstest_preallocation.jl,test_comprehensive.jl: Comprehensive test coverageValidation
Impact
This implementation delivers significant performance improvements for optimization algorithms that make frequent calls to coordinate format functions, particularly beneficial for:
Closes #392