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

Source version in Model #352

Open
bastiencarreres opened this issue Nov 3, 2022 · 6 comments
Open

Source version in Model #352

bastiencarreres opened this issue Nov 3, 2022 · 6 comments

Comments

@bastiencarreres
Copy link

bastiencarreres commented Nov 3, 2022

Hi,
First, thank you for this library!
I'm working on SN Ia simulation and I would keep track of the Source version that I can use in some meta-file to re-load the same model when needed. I have remarked that the class Model(_ModelBase) has not a version arg that can be passed to get_source. Is it something that can be implemented in a future version?

@benjaminrose
Copy link
Member

@bastiencarreres, we should be able to implement this. Do you have an example script/test that you would like to see work?

Also, @cwwalter this seems like something that might help the next round of DESC simulations as well.

@bastiencarreres
Copy link
Author

bastiencarreres commented May 25, 2023

Hi @benjaminrose , I was thinking of something simple, that could be used for fit or simulation, as for example

import sncosmo as snc
model = snc.Model('salt2', version='2.4')

In the __init__ of Models there is already that:

def __init__(self, source, effects=None,
                effect_names=None, effect_frames=None):
       # Set source and add source parameter names
       self._source = get_source(source, copy=True)

where the get_source(source, copy=True) has already a version parameter in its definition but not used here.
Is adding version in the __init__ something that could work?

def __init__(self, source, version=None, effects=None,
                effect_names=None, effect_frames=None):
       # Set source and add source parameter names
       self._source = get_source(source, version=version, copy=True)

I have tested snc.models._SOURCES.retrieve('salt2', version='2.4') and it seems to work.

@cwwalter
Copy link

Also tagging @jchiang87.

@cwwalter
Copy link

cwwalter commented Jun 9, 2023

Tagging @JoanneBogart.

@benjaminrose
Copy link
Member

@bastiencarreres, yeah, I think we should add this. This allows people to use sncosmo.Model with a string, but still fully define their source file. Currently, you would need to create a full sncosmo.Source object to do this. There are not many sources with multiple versions, so this has not come up before, but DESC is also asking for this. I think we should add a dictionary and not mess with the order of the input variables. I think we should do something like this:

def __init__(self, source, effects=None,
                effect_names=None, effect_frames=None, source_params={}):
       # Set source and add source parameter names
       self._source = get_source(source, copy=True, **source_params)

Then you can use snc.models._SOURCES.retrieve('salt2', source_params={'version': '2.4'}). It is a bit more verbose, but helps keep the default process smoother & backwards compatible.

@JoanneBogart, with this patch, we can add a similar version value to skyCatalogs's class SNObject.

@benjaminrose benjaminrose self-assigned this Jun 12, 2023
@MickaelRigault
Copy link

Hello guys, I'm also interested with this functionality. The proposition from @bastiencarreres looks like the good one to me.
We want to be able to do

model = sncosmo.Model("salt2", version="2.4")

or a parsable string (that may helps the dev as no new variables)

model = sncosmo.Model("salt2 v=2.4")

or (and this would be cleaner from the start: forbid the fact that sources may have the same Name (so rename "salt2"->salt2.4, salt2T21, salt2B22 etc)

model = sncosmo.Model("salt2.4")

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants