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

Added increment! and decrement! functions. #314

Merged

Conversation

Jay-sanjay
Copy link
Contributor

Fixes #292
updated julia_interface.jl

@abelsiqueira
Copy link
Member

Hi @Jay-sanjay, thanks for the PR. Unfortunately, that is not the increment! that we wanted - our fault for not being clear on the issue. Check it here: https://github.com/JuliaSmoothOptimizers/ADNLPModels.jl/blob/main/src/nlp.jl#L537

@Jay-sanjay
Copy link
Contributor Author

@abelsiqueira sir do check is it the way the increment!() must be done , if yes I will update it for all other quantities too , that I did earlier

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.

Hi @Jay-sanjay, thanks. That is the correct usage, but you don't have to define the functions again since they are already present in the NLPModels.jl package.

Comment on lines 5 to 27
"""
increment!(nlp, s)

Increment counter `s` of problem `nlp`.
"""
@inline function increment!(nlp::AbstractNLPModel, s::Symbol)
increment!(nlp, Val(s))
end

for fun in fieldnames(Counters)
@eval increment!(nlp::AbstractNLPModel, ::Val{$(Meta.quot(fun))}) = nlp.counters.$fun += 1
end

"""
decrement!(nlp, s)

Decrement counter `s` of problem `nlp`.
"""

function decrement!(nlp::AbstractNLPModel, s::Symbol)
setproperty!(nlp.counters, s, getproperty(nlp.counters, s) - 1)
end

Copy link
Member

Choose a reason for hiding this comment

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

These shouldn't be necessary since they are already defined in NLPModels.jl. Just remove them.

@@ -43,7 +66,7 @@ function NLPModels.objcons!(
f,
)
end
nlp.counters.neval_obj += 1
increment!(nlp, :neval_obj)
Copy link
Member

Choose a reason for hiding this comment

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

Yes, this is the correct usage.

@codecov
Copy link

codecov bot commented Oct 3, 2023

Codecov Report

All modified lines are covered by tests ✅

Files Coverage Δ
src/julia_interface.jl 99.76% <100.00%> (ø)

📢 Thoughts on this report? Let us know!.

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 @Jay-sanjay, this looks great.

@abelsiqueira abelsiqueira merged commit 237a072 into JuliaSmoothOptimizers:main Oct 3, 2023
12 checks passed
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.

Use increment! and decrement! functions
2 participants