Skip to content

Conversation

@indyalardjane
Copy link

No description provided.

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.

Thanks @indyalardjane for the PR!

I added a couple of comments.

In the functions, you don't want to convert variables with T as it will break the derivatives. We want Julia to infer the type.

Do you have bound constraints in your model as well? I.e. constraints of the form x >= lvar or x <= uvar? If so, you should prepare vector lvar and uvar and use the following constructor

ADNLPModel(f, x0, lvar, uvar, c, lcon, ucon)

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 couple of new comments.
Could you remove the two files minimalsurface.jl ? Also, can you compare your model with the Ampl version that looks simpler for some reason? Thanks!!

Comment on lines +10 to +11
using Plots
using ADNLPModels, NLPModels, NLPModelsIpopt, DataFrames, LinearAlgebra, Distances, SolverCore, PyPlot
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
using Plots
using ADNLPModels, NLPModels, NLPModelsIpopt, DataFrames, LinearAlgebra, Distances, SolverCore, PyPlot


v = vec(v_D) #convert to vector

return ADNLPModel(f, v, constraints, lcon, ucon)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return ADNLPModel(f, v, constraints, lcon, ucon)
return ADNLPModels.ADNLPModel(f, v, constraints, lcon, ucon)

Comment on lines +61 to +62
s = hx * hy * (sqrt(1 + (gi^2) +(gj)^2)) # Approximation of the derivative with the method of rectangles
S = S + s # Update the value of S
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
s = hx * hy * (sqrt(1 + (gi^2) +(gj)^2)) # Approximation of the derivative with the method of rectangles
S = S + s # Update the value of S
S = S + hx * hy * (sqrt(1 + (gi^2) +(gj)^2)) # Approximation of the derivative with the method of rectangles

Comment on lines +22 to +23
nx = 20 # number of points according to the direction x
ny = 20 # number of points according to the direction y
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason to choose 20? I suggest we do: nx = Int(round(sqrt(n))) and ny = nx

@dpo
Copy link
Member

dpo commented Jul 9, 2022

@indyalardjane It looks like there are still a few comments to address here, please.

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