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

Use the Julia wrappers in CUTEst.jl -- Part I #333

Merged
merged 2 commits into from
Aug 17, 2024

Conversation

amontoison
Copy link
Member

@amontoison amontoison commented Aug 14, 2024

  • The function set_mastsif is now in sifdecoder.jl
  • All code related to CUTEstModel is in a new file model.jl.
  • Use the Julia wrappers for fortran_open_ and fortran_close_.

@amontoison amontoison requested a review from tmigot August 14, 2024 22:03
@amontoison amontoison force-pushed the wrappers branch 2 times, most recently from b3174fb to d1a3b37 Compare August 14, 2024 22:18
@tmigot
Copy link
Member

tmigot commented Aug 15, 2024

Tests are not passing

@amontoison
Copy link
Member Author

I know...

@amontoison amontoison force-pushed the wrappers branch 4 times, most recently from 57b7cff to 7913eb2 Compare August 16, 2024 06:50
Copy link

codecov bot commented Aug 17, 2024

Codecov Report

Attention: Patch coverage is 88.74172% with 17 lines in your changes missing coverage. Please review.

Project coverage is 62.65%. Comparing base (f2ad6a4) to head (c8011e8).
Report is 20 commits behind head on main.

Files Patch % Lines
src/model.jl 85.84% 16 Missing ⚠️
src/sifdecoder.jl 94.73% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main     #333       +/-   ##
===========================================
- Coverage   89.11%   62.65%   -26.47%     
===========================================
  Files           5        8        +3     
  Lines         790     1154      +364     
===========================================
+ Hits          704      723       +19     
- Misses         86      431      +345     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@amontoison amontoison changed the title Use the Julia wrappers in CUTEst.jl Use the Julia wrappers in CUTEst.jl -- Part I Aug 17, 2024
@amontoison amontoison merged commit 97cbb72 into JuliaSmoothOptimizers:main Aug 17, 2024
11 of 15 checks passed
@amontoison amontoison deleted the wrappers branch August 17, 2024 06:54

CUTEstException(info::Integer) = CUTEstException(convert(Int32, info))

function cutest_error(status::Cint) # Handle nonzero exit codes.
Copy link
Member

Choose a reason for hiding this comment

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

Why change the macro?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because the macro was not applied to a parameter directly. It was checking a variable io_err that is defined in the same scoop of variable but outside the macro. It can break for multiple reasons (scoop, name of the variable, etc...)

@@ -0,0 +1,283 @@
mutable struct CUTEstModel <: AbstractNLPModel{Float64, Vector{Float64}}
Copy link
Member

Choose a reason for hiding this comment

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

Did things change in this file? Since you move+modify it's really hard to see

Copy link
Member Author

Choose a reason for hiding this comment

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

I updated the calls to fortran_close_ and fortran_open_ to just use the new wrappers.
I also changed vectors of one coefficient into references.
It requires less memory like this.

@@ -32,7 +32,6 @@ function main()
code = replace(code, "Ptr{ip_}" => "Ptr{Cint}")
code = replace(code, "Ptr{ipc_}" => "Ptr{Cint}")
write(path, code)

Copy link
Member

Choose a reason for hiding this comment

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

:)

Copy link
Member Author

Choose a reason for hiding this comment

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

I wanted it without the space ;)

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