LBHB / NEMS0

THIS VERSION OF NEMS IS NO LONGER SUPPORTED. PLEASE USE THE NEW NEMS REPOSITORY OR INSTALL NEMS_DB TO GET NEMS0 SUPPORT.
GNU General Public License v3.0
8 stars 4 forks source link

Migrate keywords to module and xforms libraries #171

Open svdavid opened 4 years ago

svdavid commented 4 years ago

No one loves tracking down keywords and editing them in separate files. So why not move them to the library where the corresponding functions live?

@jacobpennington idea: establish decorators for modelspec and xforms keywords. Then the keyword registries can scan all the modules and other libraries when being created. Then the keyword defs can be cleared out of plugins that dir can be reserved for users to add their own module+keywords and preproc/fitter+xforms-keywords

If we do this, we also want to make it easier to list and locate keywords and their associated code. So maybe add a few functions to nems.utils to list/search keywords?

arrrobase commented 4 years ago

I've been thinking about this a little. These are just a few ideas regarding modules/keywords/plugins I have and not fully fleshed out. Open to ideas and discussions, and I'm not attached to this in any way.

I think in terms of organization, each module should be a class. The idea would be to create a metaclass for modules with all the attributes and methods we expect modules to have, such as plot_fns, kw, parse_kw() for example. Essentially defining an interface. All modules would have to inherit from this module metaclass. We can also use the new __init_subclass_ hook in order to collect registry items.

Separating out the registry and module metaclass into separate classes allows module subclassing without registration, so there could be parent module classes to group module types and help organization.

Here's a gist example of what I'm thinking: https://gist.github.com/awctomlinson/865a94d70e8e2c4ccd4bba859761196e

I'm sure I'm not thinking of some things. Open to ideas. I think the advantage of classes would be helping logically group everything about a module together.

https://www.python.org/dev/peps/pep-0487/#subclass-registration

svdavid commented 4 years ago

Makes a lot of sense. A couple thoughts, with the goal of minimizing complexity:

arrrobase commented 4 years ago

No net increase in the number of Registries. If I understand, this is the case, as ModuleRegistry would replace the current keyword registry for modules?

I think the end goal should be to replace it, yes. This system would probably be a good way to keep track of the plotting functions as well in a PlottingRegistry or other similar, and then modules could refer to plots by keywords or names rather than pathspec, to allow a similar overriding of plotting functions by end users.

Also add a way of producing a TensorFlow "layer" as defined in nems.tf.cnnlink.modelspec2tf

This would probably not require much rewriting. The only thing I can see changing would be the ['fn'] attribute in the modelspecs. This brings up an important point though. If the paths to all of the modules change, then the modelspecs/xforms may no longer be backwards compatible. We could add an attrribute like old_path or something to address this.

Q: Embed plot_fn/plotting routines in the module rather than modelspec? Or just use the module to list plot functions? (as is done now)

I think keeping the plotting functions separate makes the most sense, to allow reuse the way the current system is set up. It might be good to be consistent in what arguments these plotting functions accept, and perhaps the modules could have a plotting_args attribute so they could define how exactly they wanted to use each plot (or at least defaults), instead of leaving it up to quickplot or something else.

Can fn_kwargs simply be properties of the module?

Instead of being saved in the modelspec?