Skip to content

Conversation

@arnavk23
Copy link
Contributor

@arnavk23 arnavk23 commented Nov 17, 2025

@arnavk23
Copy link
Contributor Author

@tmigot please review. On my system they are passing when I use one(T), T(i) and T(mod(i,5)).

Copy link
Member

@tmigot tmigot left a comment

Choose a reason for hiding this comment

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

A first batch of comments

@arnavk23
Copy link
Contributor Author

@tmigot Worked on your review comments but now seeing

  Expression: typeof(obj(nlp, x0)) == T
   Evaluated: Float64 == Float32

@arnavk23 arnavk23 requested a review from tmigot November 18, 2025 22:38
Copy link
Member

@tmigot tmigot left a comment

Choose a reason for hiding this comment

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

@arnavk23 I stopped after the first file

@arnavk23 arnavk23 requested a review from tmigot November 24, 2025 00:57
Copilot AI review requested due to automatic review settings December 17, 2025 06:34
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request adds three new optimization test problems (Problems 9, 10, and 16) from the Luksan, Matonoha, and Vlcek test problem collection. The implementation follows the established pattern for optimization problems in this repository, providing three variants: PureJuMP models, ADNLPProblems models, and metadata files.

Key Changes:

  • Added three trigonometric optimization problems: trig (Problem 9), toint (Problem 10), and trigb (Problem 16)
  • Each problem is implemented in three formats: PureJuMP, ADNLPProblems, and metadata configurations
  • All problems are unconstrained minimization problems with variable dimensions

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/PureJuMP/trig.jl PureJuMP implementation of the "Another trigonometric function" problem (Problem 9)
src/PureJuMP/toint.jl PureJuMP implementation of the "Toint trigonometric function" problem (Problem 10)
src/PureJuMP/trigb.jl PureJuMP implementation of the "Banded trigonometric problem" (Problem 16)
src/ADNLPProblems/trig.jl ADNLPProblems implementation of Problem 9 using automatic differentiation
src/ADNLPProblems/toint.jl ADNLPProblems implementation of Problem 10 with critical bugs in division operators
src/ADNLPProblems/trigb.jl ADNLPProblems implementation of Problem 16
src/Meta/trig.jl Metadata configuration for the trig problem
src/Meta/toint.jl Metadata configuration for the toint problem
src/Meta/trigb.jl Metadata configuration for the trigb problem

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@arnavk23 arnavk23 requested a review from tmigot December 17, 2025 06:46
@codecov
Copy link

codecov bot commented Dec 17, 2025

Codecov Report

❌ Patch coverage is 51.00671% with 73 lines in your changes missing coverage. Please review.
✅ Project coverage is 54.34%. Comparing base (2ada450) to head (d566d21).
⚠️ Report is 43 commits behind head on main.

Files with missing lines Patch % Lines
src/PureJuMP/toint.jl 0.00% 30 Missing ⚠️
src/PureJuMP/trig.jl 0.00% 26 Missing ⚠️
src/PureJuMP/trigb.jl 0.00% 5 Missing ⚠️
src/Meta/toint.jl 33.33% 4 Missing ⚠️
src/Meta/trig.jl 33.33% 4 Missing ⚠️
src/Meta/trigb.jl 33.33% 4 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main     #396       +/-   ##
===========================================
- Coverage   99.80%   54.34%   -45.47%     
===========================================
  Files        1084     1138       +54     
  Lines       12319    13371     +1052     
===========================================
- Hits        12295     7266     -5029     
- Misses         24     6105     +6081     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@@ -0,0 +1,40 @@
export toint

function toint(; n::Int = default_nvar, type::Type{T} = Float64, kwargs...) where {T}
Copy link
Member

Choose a reason for hiding this comment

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

There seems to be an inconsistency between the AD version and the JuMP version:

julia> ju_nlp1 = MathOptNLPModel(OptimizationProblems.PureJuMP.toint());

julia> ad_nlp1 = OptimizationProblems.ADNLPProblems.toint();

julia> obj(ad_nlp1, ad_nlp1.meta.x0) == obj(ju_nlp1, ju_nlp1.meta.x0)
false

The other two seem correct.

@arnavk23 arnavk23 requested a review from tmigot December 20, 2025 05:22
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