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.

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

Re-export a few generics and types from the TypedTables package #662

Open
dmbates opened this issue Jan 9, 2023 · 7 comments
Open

Re-export a few generics and types from the TypedTables package #662

dmbates opened this issue Jan 9, 2023 · 7 comments

Comments

@dmbates
Copy link
Collaborator

dmbates commented Jan 9, 2023

I am finding it quite convenient to work with the Table type from TypedTables.jl and have added a dependency on that package in the profile branch. I plan to re-export a few generics and types like Table and group and, maybe, groupinds from MixedModels itself. This is just a heads-up and request for comment in case someone thinks this is a terrible idea.

@dmbates
Copy link
Collaborator Author

dmbates commented Jan 9, 2023

@palday, what would you think of changing the structure of the fitlog to be a

Vector{NamedTuple{(:θ, :obj),(NTuple{K,T}, T)}} so that it can be displayed as a Table? If I understand correctly using an NTuple{K,T} instead of a Vector{T} to store each parameter vector in the fit log should even save a bit of storage.

@dmbates
Copy link
Collaborator Author

dmbates commented Jan 9, 2023

I am also considering changing the format of the allpars property of a parametric bootstrap to be like that of the tbl property of a MixedModelProfile. That is, a Vector{<:NamedTuple} which can be displayed as a Table. @palday, I would defer to you regarding the implications of such changes for version numbers under semantic versioning. It is probably best to save up several such incompatible changes for one, massive, break with the past. It is such a pity that we have to go through the stages of "keep doing it wrong until you do it right" in this type of design.

@dmbates
Copy link
Collaborator Author

dmbates commented Jan 9, 2023

An alternative for the parametric bootstrap is just to introduce a new property, say tbl, which has the Table-compatible structure and deprecate the allpars property.

@palday
Copy link
Member

palday commented Jan 10, 2023

Ergonomic design is only "obvious" in retrospect.

We do a decent job of keeping release notes and haven't had a breaking release in a while, so I'm not terribly opposed to having a clean-up cycle + major release in the near future. If the breaking changes don't impact a particular user, then they can feel free to have MixedModels = "4, 5" in their compat entry.

That said, I haven't yet thought about the proposal. I would suggest implementing it and seeing how much we like it for a little bit. As soon as we have something breaking on main, I can split off a release-4.x branch for any small things we want to backport. Then we can tinker on main for a little while before tagging 5.0 (and I can get to work on all the breaking changes I want to do in 5.0, such as some consistency stuff and stripping out the multithreading -- it's better to let the BLAS do multithreading and maybe add a nice example of doing multiprocessing to the docs).

@dmbates
Copy link
Collaborator Author

dmbates commented Jan 26, 2023

Another couple of things to throw in the hopper for a breaking version is to change the name and default value of the argument to fit that causes the fitlog to be created. In retrospect, calling this thin was inappropriate. I, at least, have never used it as other than a toggle to keep the fitlog or not so the concept of "thinning" the fitlog and keeping only every 10'th value is not particularly valuable to me. If one really wanted that then it would be possible to post-process the fitlog. I doubt that the amount of memory required to keep the entire fitlog is ever going to be the limiting factor for fitting a model.

I am also thinking of changing the structure to be a row-oriented table (Vector{NamedTuple}) with the first column being the objective and the remaining columns being the values of the theta parameters.

@palday
Copy link
Member

palday commented Jan 26, 2023

see also #618

@dmbates
Copy link
Collaborator Author

dmbates commented Jan 26, 2023

Exactly. I am getting to the point where I keep coming up with the same ideas, over and over, and considering them to be novel approaches. I suppose in a way that this is better than coming up with contradictory ideas and considering them to be worthwhile, novel approaches.

@dmbates dmbates mentioned this issue Feb 10, 2023
3 tasks
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

No branches or pull requests

2 participants