Define StructType for OptSummary

[dmbates]

• add dependencies on StructTypes and JSON3
• define StructType and excludes for OptSummary
• still needs tests

[codecov]

Codecov Report

Merging #506 (b2ce503) into main (7b6dd31) will increase coverage by 0.03%.
The diff coverage is 97.05%.

❗ Current head b2ce503 differs from pull request most recent head 1a812c3. Consider uploading reports for the commit 1a812c3 to get more accurate results

@@            Coverage Diff             @@
##             main     #506      +/-   ##
==========================================
+ Coverage   93.98%   94.02%   +0.03%     
==========================================
  Files          26       26              
  Lines        2162     2191      +29     
==========================================
+ Hits         2032     2060      +28     
- Misses        130      131       +1     

Impacted Files	Coverage Δ
src/linearmixedmodel.jl	95.47% <96.15%> (+0.02%)	⬆️
src/likelihoodratiotest.jl	97.67% <100.00%> (+0.05%)	⬆️
src/optsummary.jl	100.00% <100.00%> (ø)

---

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 7b6dd31...1a812c3. Read the comment docs.

[palday]

If we define a stable serialization format and a stable set of properties, we might consider making the set of fields an implementation detail, in which case a lot of the breaking optimizations we've done lately would no longer be breaking.

[palday]

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

[dmbates]

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.

[palday]

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.

[dmbates]

@palday I plan to add a save/restore pair and have been thinking about what to call them. I am leaning toward signatures like saveoptsum(io::IO, m::MixedModel) and restoreoptsum(m::MixedModel, io::IO). I will start with just the LinearMixedModel. I am not wedded to these names and this design.

[palday]

I don't have a better idea for names right now (and I think we should probably avoid using save or restore in exported functions), but maybe @ararslan or @kleinschmidt do? They both seem to have a knack for detecting unfortunate naming conventions.

But: the restore function needs a bang, doesn't it (like fit!)?

[dmbates]

@palday Agree that the name of the restore function should end with !.

[dmbates]

I'm not sure that I am being sufficiently paranoid about the tests built into restoreoptsum! here but this should be a start.

[palday]

Looks good. Thoughts for future PRs:

1. GLMM. I think it actually should be pretty easy to do.
2. using the same logic here to update a model to have the fit from a particular bootstrap iteration
3. serializing bootstrap this way

(Future PRs because I suspect the devil is in the details for all of those.)