astropy / saba

A Package which allows astropy to interface with sherpa
GNU General Public License v3.0
8 stars 9 forks source link

Docs example how to translate model? #9

Open cdeil opened 8 years ago

cdeil commented 8 years ago

@nocturnalastro - First of all - thank you for this Sherpa - Astropy bridge!

I presume you have some machinery to translate between Sherpa and Astropy models?

The current docs only talk about SpectrumFitter and never about the model on the Sherpa side? http://saba.readthedocs.io/en/latest/index.html

Could you write up some info (maybe with a code example) how this works? I.e. show the attributes and structure of the models on each side with an example and explain how the (fully auto?) translation works?

Some background: In Gammapy we currently have a mix of models that are Astropy, Sherpa and independent, and some code to translate to Sherpa to fit them, e.g. here: https://github.com/gammapy/gammapy/blob/master/gammapy/spectrum/models.py#L246 It's a bit of a mess and the implementation isn't good. One option would be that we implement all models using Astropy (mainly because it's a core dependency of Gammapy, not everyone has Sherpa) and use your bridge for fitting with Sherpa.

I plan to look at what you've built here and try it out in the coming days / weeks.

olaurino commented 8 years ago

@cdeil the project was focused on enabling Astropy users to use the Sherpa optimizers, statistics, and confidence estimators.

However, there is a proof of concept on how to import the Astropy models in Sherpa as PR sherpa/sherpa#239

Also, while documentation is improved, did you take a look at the tests?

nocturnalastro commented 8 years ago

Hi @cdeil thanks, I enjoyed making it. The conversion between Astropy and Sherpa models occurs in the ConvertedModel class, we generate Sherpa user models with the same parameters (see line 673) as the astropy models and then link th sherpa models calc method to astropy models evaluate method (see line 658) as the do the equivalent job in there respective packages - except the data and the parameters are supplied in a different order.

I'll try to get the docs updated but I'm away this weekend so it may be a week or so before I get round to it.

I've just looked at the API issue and I just exported the API image again (inkscape hates me apparently), I'll merge it soon.

How this helps.

cdeil commented 8 years ago

So is it worth exposing ConvertedModel as part of the saba API for people like me that want to translate Astropy <-> Sherpa models back and forth?

Or is that a temp solution that will be superseded by https://github.com/sherpa/sherpa/pull/239 and work in the Sherpa repo?

olaurino commented 8 years ago

@cdeil the idea in Saba is that you don't need to have an explicit translation. As long as you have an Astropy model (and you don't hit any limitations in the Saba mapping), you can just instantiate a SherpaFitter with the characteristics you need, pass it your model and all the data, and get the results back. Sherpa optimization and statistics will be called for you using Sherpa classes, under the hood.

You would need a translation, at this point, if you wanted to use Astropy models directly from the Sherpa UI. To some extent, i.e. if the models are registered in Astropy, you could get that for free eventually, but the code is not there yet.

Or am I missing something?

taldcroft commented 8 years ago

@olaurino - I think I see where @cdeil is going in that we have been focusing on a target audience of astropy users to make it easier to use the astropy fitting API from start to finish (model definition through calling a fitting object). But if you have experienced Sherpa users that want to use astropy models within the Sherpa UI then exposing ConvertedModel might make that easy.

It looks like one should be able to just create a ConvertedModel object and access the sherpa_model attribute and it's done. I just don't know how much that has been tested outside the context of saba.

nocturnalastro commented 8 years ago

ConvertedModel is available from saba.main. @taldcroft is correct if you instantiate you can access the sherpa_model then it might be a case of registering it inside the UI as shown in Tom's proof of concept or using add_model. I'm not sure which is the correct method you guy's probably know more than me about this.

nocturnalastro commented 7 years ago

I just remembered when going through these, I started some code to allow access to astropy model from sherpa. I haven't tested it! It can be found here. After I've finished writing my thesis I'll take another look into this.