Make creating new simple models more user-friendly

Right now creating a model in models can be a bit complicated, as it requires overriding a few different methods. Many common models can be expressed quite easily as just a python function (that accepts arrays and parameters). So I suggest creating a metaclass or possibly a function decorator that can be used to build a new model class from just a single function. (The sort of thing I have in mind is something like http://pythonhosted.org/PyModelFit/over.html#creating-a-custom-model )

Additionally, the documentation on creating a new model is probably a bit terse for users (although maybe those more in the developer camp). So making a few more extended examples might be helpful so that users can make whatever kinds of complex models they might want.

(This refers to the models subpackage that will be in astropy when #493 is merged. The intent is to merge that without significant design changes, and address proposed updates through additional issues/PRs like this one)

I'll probably work on this myself when I get a chance, but if someone else feels inclined to take a crack at it, they can feel free.

@eteq

@eteq: Here’s an example how to automatically get parameter names from a Python callable, just in case that is what you had in mind:

You can see some examples of how it is used here

@nden

Just to be clear that I'm not being secretive about this, I've been working over the past couple days on a slight tweaking of the models API that I think will support this better. Though it might conflict with some of the changes @nden has been making too, so I'm trying to tread lightly here. If nothing else my changes, if they work, can act as a proposal.

@iguananaut

Ok, then I'll wait on doing anything about this until I see what you had in mind, @iguananaut . You might want to check out the results of the models telecon though and make sure you're not interfering with the changes proposed there (I'll send you the link via e-mail - @nden may not have wanted the doc public, I'm not sure).

I don't think it will really interfere/overlap with too much of the other things that are happening. The problem I'm trying to address is that descriptors aren't really used to their full advantage, and the overprofilration/overcomplication of Parameter objects. In other words I'm trying to make the handling of Parameters simpler and more streamlined. I'll probably have something for that by the end of the week. The code will almost certainly conflict with some of the other PRs that resulted from that meeting but if my proposed changes look acceptable I can rebase them against the other changes no problem. I don't think they conflict conceptually for the most part.

I'm almost ready with this, but I have a few more things that need to get working, as well as integrating with the new work done in #1075. But the work I'm doing should simplify Parameters even further, while also making them possibly more powerful and descriptive assuming I can get everything working.

With this it will probably be able to create some kind of model_factory() function that can turn any function into a model as described in the OP.

I just tried to add some more unit tests for better coverage of parameter.py in #1060 and noticed that to create a Parameter I need a Model instance. Instead of trying to understand why that is needed from the code I'll just ask: is this tight coupling really necessary?

I briefly looked at the Sherpa Parameter class here and if I understand correctly it also has a reference to it's model, but only via the model name string, so a Sherpa Parameter can be created without having a Model instance.

In any case the current Parameter.mclass is a misnomer, because Parameter.mclass is not a model class, but an instance, so if Parameter really needs this reference back to its model, can we rename the property to Parameter.model?

I moved this comment to #1078

@iguananaut

I haven't seen @iguananaut 's changes yet but to keep things separate and more manageable it may be better to create a different issue about parameters. This one is already overloaded.

Most of these are bugs that can be fixed quickly.
The exceptionn is the reference to mclass in Parameter which is needed for handling constraints. I've been tripped by the same thing when testing interactively but didn't think a user will ever need to do this. It may be possible to rework constraints so that the Parameter object does not need a reference to mclass but this will be a bigger change and I need to think more about it.

@cdeil

@cdeil I agree. I already changed that. I will try to post my changes today as soon as I have all the existing tests passing so that we can have something more concrete to talk about.

What I'm actually doing is trying to simplify things by having all pertinent data stored on the model object itself. The Parameter and Parameters objects just provide special views to those parameters, but all the actual data specific to the model is stored in just one place instead of proliferated across several objects.

@adonath

@adonath - are you interested in pursuing this as part of your GSoC project? This will make #981 much easier, so it should be done before implementation on #981 begins.

See pull request #1203

@nden

Please also don't forget about #1086, which overhauls the model API quite a bit, though I think it would be compatible with most of the changes in #1203 (which, AFAICT, mostly works by breaking up some repetitive code into more model subclasses).

I still need to address some concerns @nden had with my approach, and I haven't had time to do so lately, but I just wanted to point out that I haven't abandoned my proposals in that PR.

Simplified creating new models astropy#980

@eteq

@eteq @adonath @cdeil Neat, I hadn't even thought of making it a decorator. I'll keep that in mind (I would use a simpler name though--remember decorators can take arguments).

ETA: In that vein since the CustomNDModel classes can be used as decorators I take back, somewhat, my misgivings about the approach. That said, I feel that it should still be changed to return a new class, and not actually an instance of CustomNDModel.

@cdeil Also +1 to not using this in Astropy itself. Too much hassle from a maintenance point of view. But it's definitely useful as a convenience for users.

@adonath

@adonath @cdeil - actually, I was thinking that it would be nice to use this to implement the core models, although I hadn't actually suggested it yet. But I see your points on this, and clearly I'm outvoted, anyway :)

I also strongly agree with @embray that the decorator should yield a class object, not an instance. I want users to be able to do:

@Custom1DModel
def LineModel(x, m=1,b=0):
    return m * x + b

m1 = LineModel()
m2 = LineModel(m=2,b=4.3)

and similar, otherwise it's not much of a convenience. My thinking is that this would be used interactively and in quick science code for those who just want an easy-to-implement function-based model without having to understand too much of the details of modeling (unless I missed something/the current docs aren't up to date, it's still not nearly that simple if you go the subclass road).

I'm hoping to have more time to poke at this but I'm spread pretty thin at the moment...

@eteq

Over in #1086, which I just updated, I experimented with replacing the Custom1DModel with a @custom_model_1d decorator quite similar to @eteq's example above: https://github.com/astropy/astropy/pull/1086/files#L7R1160

This could still use a lot of enhancement, but the basic concept is sound, and seems to work.

@embray

Looks promising, @embray - if that gets merged I think this can get closed (I will review it soon)

@nden

FWIW #1086 still needs some work to address some of the points @nden brought up earlier, and I need to coordinate with her in general but she's out of town. If people like my custom model approach, however, I can easily pull that out into a separate PR as well. Most of it is not terribly dependent on my other reforms (though a little of it might be).

I'm going to close this on the grounds that there are larger API changes coming and nothing specific about this beyond #1086 would make 0.3, and by 0.4 I'm sure there will be other API changes to think about. But there might be a new issue on this, depending on where we stand in a few months.

As discussed in astropy#980, rewrote Custom1DModel as a decorator tha…

Navigation Menu

Repository navigation

Make creating new simple models more user-friendly #980

12 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Make creating new simple models more user-friendly #980

Description

Activity

eteq commented on Apr 13, 2013

cdeil commented on May 6, 2013

embray commented on May 7, 2013

eteq commented on May 8, 2013

embray commented on May 8, 2013

embray commented on May 9, 2013

cdeil commented on May 10, 2013

cdeil commented on May 10, 2013

nden commented on May 10, 2013

embray commented on May 10, 2013

eteq commented on Jun 14, 2013

adonath commented on Jun 24, 2013

embray commented on Jun 25, 2013

12 remaining items

embray commented on Aug 14, 2013

eteq commented on Sep 4, 2013

embray commented on Sep 4, 2013

embray commented on Sep 18, 2013

eteq commented on Sep 23, 2013

embray commented on Sep 23, 2013

eteq commented on Oct 18, 2013

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions