Skip to content

Conversation

@MohamedLaghdafHABIBOULLAH
Copy link
Contributor

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 @MohamedLaghdafHABIBOULLAH ! It already looks very good, I only have a couple of small comments. Please, check duct12 and duct15 as there seem to be some problems with these two.

end

function f(y)
return sum(nfrob(e,y)/(3*area(e,y)^(2/3)) for e=1:E)
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 sum(nfrob(e,y)/(3*area(e,y)^(2/3)) for e=1:E)
return sum(nfrob(e,y)/(3*area(e,y)^(2//3)) for e=1:E)

ensures type stability.


include("../../data/tetra_duct12.jl")
export tetra_duct12
tetra_duct12(;kwargs...) = tetra(xe_duct12, TETS_duct12, Const_duct12; kwargs...)
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
tetra_duct12(;kwargs...) = tetra(xe_duct12, TETS_duct12, Const_duct12; kwargs...)
tetra_duct12(;kwargs...) = tetra(xe_duct12, TETS_duct12, Const_duct12; name = "tetra_duct12", kwargs...)

Can you also add names for the others?


include("../../data/tetra_duct15.jl")
export tetra_duct15
tetra_duct15(;kwargs...) = tetra(xe_duct15, TETS_duct15, Const_duct15; kwargs...)
Copy link
Member

Choose a reason for hiding this comment

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

It seems that xe_duct15 is a matrix and not a vector


include("../../data/tetra_duct12.jl")
export tetra_duct12
tetra_duct12(;kwargs...) = tetra(xe_duct12, TETS_duct12, Const_duct12; kwargs...)
Copy link
Member

Choose a reason for hiding this comment

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

Also, for some reason, this one returns an error. Can you verify?

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.

I think I found why the tests were failing, we should give a specific name even for the default data.

MohamedLaghdafHABIBOULLAH and others added 6 commits June 7, 2022 14:28
Co-authored-by: tmigot <tangi.migot@gmail.com>
Co-authored-by: tmigot <tangi.migot@gmail.com>
Co-authored-by: tmigot <tangi.migot@gmail.com>
Co-authored-by: tmigot <tangi.migot@gmail.com>
Co-authored-by: tmigot <tangi.migot@gmail.com>
Co-authored-by: tmigot <tangi.migot@gmail.com>
@codecov
Copy link

codecov bot commented Jun 7, 2022

Codecov Report

Merging #190 (2134c68) into main (6161ae6) will decrease coverage by 0.63%.
The diff coverage is 42.46%.

@@            Coverage Diff             @@
##             main     #190      +/-   ##
==========================================
- Coverage   99.89%   99.25%   -0.64%     
==========================================
  Files         724      741      +17     
  Lines        6401     6589     +188     
==========================================
+ Hits         6394     6540     +146     
- Misses          7       49      +42     
Impacted Files Coverage Δ
src/Meta/tetra.jl 0.00% <0.00%> (ø)
src/Meta/tetra_duct12.jl 0.00% <0.00%> (ø)
src/Meta/tetra_duct15.jl 0.00% <0.00%> (ø)
src/Meta/tetra_duct20.jl 0.00% <0.00%> (ø)
src/Meta/tetra_foam5.jl 0.00% <0.00%> (ø)
src/Meta/tetra_gear.jl 0.00% <0.00%> (ø)
src/Meta/tetra_hook.jl 0.00% <0.00%> (ø)
src/ADNLPProblems/tetra.jl 100.00% <100.00%> (ø)
src/Meta/triangle_pacman.jl 100.00% <0.00%> (ø)
src/Meta/triangle_deer.jl 100.00% <0.00%> (ø)
... and 7 more

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 6161ae6...2134c68. Read the comment docs.

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.

@tmigot tmigot merged commit 0eb18a3 into JuliaSmoothOptimizers:main Jun 8, 2022
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