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

Add support for view inputs #269

Merged
merged 4 commits into from
Sep 1, 2021

Conversation

sshin23
Copy link
Contributor

@sshin23 sshin23 commented Aug 28, 2021

Hey JSO devs, we've been using CUTEst.jl for benchmarking our solver MadNLP.jl, and we recently found that JSO's current julia interface does not efficiently handle view inputs. Please consider merging this PR.

This PR adds better support for view inputs for CUTEst.jl's julia interface.

The current CUTEst.jl julia interface converts any AbstractVector input to Vector{Float64} to make it compatible with subsequent ccalls, and this process accompanies memory allocations. But when the AbstractVector is a stride-one array (which typically comes from a view of a Vector over a UnitRange), this can be avoided by directly passing it to C functions.

I have added the definition for StrideOneVector{T}, and replaced the type specifications in the functions from ::Vector{Float64} to ::StrideOneVector{Float64}. This change allows directly using stride-one vectors as inputs to the C functions instead of allocating more memory. I have also added a test for StrideOneVector{T} in test_core.jl.

@codecov
Copy link

codecov bot commented Aug 29, 2021

Codecov Report

Merging #269 (15f3ea3) into main (80685c6) will not change coverage.
The diff coverage is 82.60%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #269   +/-   ##
=======================================
  Coverage   84.61%   84.61%           
=======================================
  Files           5        5           
  Lines         598      598           
=======================================
  Hits          506      506           
  Misses         92       92           
Impacted Files Coverage Δ
src/CUTEst.jl 83.52% <ø> (ø)
src/core_interface.jl 72.85% <78.94%> (ø)
src/julia_interface.jl 99.58% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 80685c6...15f3ea3. Read the comment docs.

@dpo dpo requested a review from abelsiqueira August 29, 2021 18:25
Copy link
Member

@dpo dpo left a comment

Choose a reason for hiding this comment

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

Many thanks for this. Just a few rather unsubstantial comments for now.

src/CUTEst.jl Outdated Show resolved Hide resolved
src/CUTEst.jl Outdated Show resolved Hide resolved
src/CUTEst.jl Outdated Show resolved Hide resolved
sshin23 and others added 3 commits August 30, 2021 21:43
Co-authored-by: Dominique <[email protected]>
Co-authored-by: Dominique <[email protected]>
Co-authored-by: Dominique <[email protected]>
Copy link
Member

@abelsiqueira abelsiqueira left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, very good stuff!

@dpo dpo merged commit 6ea3043 into JuliaSmoothOptimizers:main Sep 1, 2021
@dpo
Copy link
Member

dpo commented Sep 1, 2021

thank you!

@sshin23
Copy link
Contributor Author

sshin23 commented Sep 2, 2021

thanks for the merge!

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.

3 participants