-
Notifications
You must be signed in to change notification settings - Fork 102
[ENH] - Add explicit {aperiodic, periodic} 'Modes' support #298
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
Conversation
@ryanhammonds & @voytek - tagging you here so you can get an eye on this, to see where I'm trying to move things here, and how this relates to #291. It's not ready for a review in any real way - but thoughts (especially "yay" / "nay" votes on this direction) are appreciated! |
On the surface this looks good. I can't quite tell, but does this construction allow for something like the dual-knee fitting approach we've been playing with, for example? |
@voytek - yeh, the goal is that it extends to this - the Mode definition would include the function and list of parameters, and then the rest of the code would read this definition to dynamically updates things like the expected number of parameters and labels, etc. That part of it isn't built in yet - but I don't currently foresee any major roadblocks to doing it! If this looks good as a first draft / direction - I can keep working to get to some proof of concepts that get a bit further and show support for an actually new / different fit function! |
Cool, this looks like it track's important/relevant metadata for new models. What is the plan for handling parameter guess procedures? I guess that's kinda related to #291. I guess for each parameter of new models model there could be either a hard-coded guess/bounds or new procedures to generate the guess/bounds. |
This PR has become a bit of a beast, and I don't think there's much utility in trying to review the extensive diffs. The updates here are now all done in way that everything works for using modes - with a bit more work probably to come on a bit more clean up and generalization. I'm going to merge this in now, and we can go from there for further work. |
This PR updates to use fit "modes" to define and use
aperiodic_mode
andperiodic_mode
as flexible fit modes that not only define and support a way to add new fit modes to the model, but also set up the infrastructure to allow for passing in fit functions. In addition, it adds a lot of updates and developments along the way, perhaps most notably further updating the model object organization to use sub-objects instead of multiple inheritance."Fit Mode" idea
The notable update re-modes is
specparam.modes
, where there is a definition of functionality to define a fit mode. For a brief overview, the base idea is that for each of the {aperiodic, periodic} modes, we define a "Mode" for how we fit this. Specifically, this is basically the fit function itself, but also, we define a set of associated meta-data that describes the parameters and so on. The core of this update is to update a lot of hard-coded stuff across the code, so that as much as possible it reads needed information dynamically from theMode
object, allowing for flexibility (across things like accessing parameters, printing results, reports, plots, IO, manipulating objects). By using thisMode
information, we don't have to hard-code information about number of parameters / labels, etc, and can read this on the fly from the Mode definition.Passing in fit functions
In addition, this also allows for the basics of passing in a new fit function (note that this is currently a basic proof of concept).
This is a simple example (doesn't change much about the number of parameters, etc), but is a simple example to show the idea.
Related Updates in this PR
This PR also continues to model object reorganization started in #291 (and partly in #354) - most notably to keep the 'sub-objects' organization developed there, but now to add sub-objects to the Model object, rather than multiple inheritance that attaches all the attributes to the object at the same level. This changes the API a bit, but most of the main user-facing functionality is updated to stay the same, with some more nuanced access points now needing updates (e.g. going from Model.n_peaks_ -> Model.results.n_peaks_).
There is also a wide range of associated changes in the PR, including updating to use the
Modes
approach across:Going Forward
If you explore the Mode object, there are some aspects that look further forward, in particular defining the data spacing. The idea is that a particular mode can be defined on a particular spacing (for each of 'freqs' and 'powers'), and so by defining this, and ultimately setting and checking this in relating to the Data object, one can define arbitrary fit functions across arbitrary data representations. When integrated with allowing for arbitrary fit procedures from #291, we get the full level of generalization!