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.

Sign up for GitHub

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

Define StructType for OptSummary #506

Merged
merged 7 commits into from
Apr 13, 2021
Merged

Define StructType for OptSummary #506

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
29edcc2
Define StructType for OptSummary
dmbates Apr 11, 2021
77e2741
Add test for JSON form of OptSummary
dmbates Apr 12, 2021
9d9261f
Add and export saveoptsum, restoreoptsum! and test
dmbates Apr 12, 2021
0c06927
Correction in a test
dmbates Apr 12, 2021
22f3a52
Prune some dependencies.
dmbates Apr 12, 2021
6bdeb62
Ensure second test is on a freshly-defined model instance
dmbates Apr 13, 2021
1a812c3
Reformulate restoreoptsum! with better checks.
dmbates Apr 13, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions Project.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ Arrow = "69666777-d1a9-59fb-9406-91d4454c9d45"
DataAPI = "9a962f9c-6df0-11e9-0e5d-c546b8b5ee8a"
Distributions = "31c24e10-a181-5473-b8eb-7969acd0382f"
GLM = "38e38edf-8417-5370-95a0-9cbb8c7f171a"
JSON3 = "0f8b85d8-7281-11e9-16c2-39a750bddbf1"
LazyArtifacts = "4af54fe1-eca0-43a8-85a7-787d91b784e3"
LinearAlgebra = "37e2e46d-f89d-539d-b4ee-838fcccc9c8e"
Markdown = "d6f4376e-aef5-505a-96c1-9c027394607a"
Expand All @@ -21,13 +22,15 @@ Statistics = "10745b16-79ce-11e8-11f9-7d13ad32a3b2"
StatsBase = "2913bbd2-ae8a-5f71-8c99-4fb6c76f3a91"
StatsFuns = "4c63d2b9-4356-54db-8cca-17b64c39e42c"
StatsModels = "3eaba693-59b7-5ba5-a881-562e759f1c8d"
StructTypes = "856f2bd8-1eba-4b0a-8007-ebc267875bd4"
Tables = "bd369af6-aec1-5ad0-b16a-f7cc5008161c"

[compat]
Arrow = "1"
DataAPI = "1"
Distributions = "0.21, 0.22, 0.23, 0.24"
GLM = "1"
JSON3 = "1"
LazyArtifacts = "1"
NLopt = "0.5, 0.6"
PooledArrays = "0.5, 1"
Expand All @@ -36,6 +39,7 @@ StaticArrays = "0.11, 0.12, 1"
StatsBase = "0.31, 0.32, 0.33"
StatsFuns = "0.8, 0.9"
StatsModels = "0.6"
StructTypes = "1"
Tables = "1"
julia = "1.4"

Expand Down
2 changes: 2 additions & 0 deletions src/MixedModels.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ using Arrow
using DataAPI
using Distributions
using GLM
using JSON3
using LazyArtifacts
using LinearAlgebra
using Markdown
Expand All @@ -16,6 +17,7 @@ using StaticArrays
using Statistics
using StatsBase
using StatsModels
using StructTypes
using Tables

using LinearAlgebra: BlasFloat, BlasReal, HermOrSym, PosDefException, copytri!
Expand Down
3 changes: 3 additions & 0 deletions src/optsummary.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,3 +110,6 @@ function NLopt.Opt(optsum::OptSummary)
end
opt
end

StructTypes.StructType(::Type{<:OptSummary}) = StructTypes.Mutable()
Copy link
Member

Choose a reason for hiding this comment

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

Since OptSummary is a concrete type, there can't be subtypes and the subtype operator can't really do anything.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The subtype operator is necessary because OptSummary is a parametric type. The concrete types are OptSummary{Float64}, etc. I had drafted an issue on whether it makes sense to continue to have OptSummary be a parametric type - it is closely tied to the NLopt package for which the only floating point type that can be used is Float64 - but I didn't post it.

Anyway, the choices are to use the subtype operator here or to remove the parameter for OptSummary and I chose to keep the parameter. It's a house of cards in a way - if we remove the parameter on OptSummary we might as well remove it everywhere. I think it is better to retain the redundant parameter on OptSummary and modify that struct if we decide to incorporate other optimizers.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, yeah I had forgotten that OptSummary is parametric. I think it's worth keeping around the type parameter. On my long list of dream projects is changing the internals to take advantage of MathOptInterface, which I think NLopt finally supports. Then we could trivially support additional optimizers.

StructTypes.excludes(::Type{<:OptSummary}) = (:lowerbd, )