Open sidneymau opened 6 years ago
After some digging, ugali/analysis/mcmc.py
references ugali/analysis/loglike.py
, where the source is made here. It looks like config.get(section) != None
, but config.get(section).get('source') == None
, so it ends up reading params == None
. If this function is forced to use params = config.get('source')
, then the params end up loading properly and we get out a g-i
isochrone. It's not clear to me whether this is the expected behavior, or if we want to throw an error when getting the section is not none but getting the source from the section is none. Disregarding that, a simple fix is writing the function like:
def createSource(config, section=None, **kwargs):
config = Config(config)
source = Source()
if (config.get(section) and config.get(section).get('source')) is not None:
params = config.get(section).get('source')
else:
params = config.get('source')
if params is not None:
source.load(params)
source.set_params(**kwargs)
return source
This lead to some other pieces of ugali breaking... suggesting that support for bands other than g-r
isn't universal, or I'm missing some masks:
Traceback (most recent call last):
File "./ugali/analysis/mcmc.py", line 352, in <module>
like = ugali.analysis.loglike.createLoglike(config,source)
File "/home/s1/smau/software/ugali/ugali/analysis/loglike.py", line 600, in createLoglike
observation = createObservation(config,lon,lat)
File "/home/s1/smau/software/ugali/ugali/analysis/loglike.py", line 549, in createObservation
mask = createMask(config,roi)
File "/home/s1/smau/software/ugali/ugali/analysis/loglike.py", line 564, in createMask
mask = ugali.observation.mask.Mask(config, roi)
File "/home/s1/smau/software/ugali/ugali/observation/mask.py", line 52, in __init__
self._photometricErrors()
File "/home/s1/smau/software/ugali/ugali/observation/mask.py", line 349, in _photometricErrors
pars_2 = MAGERR_PARAMS[release][band_2]
KeyError: 'i'
Didn't we talk about that last issue before? The updates to MAGERR_PARAMS
would need to be in ugali/utils/constants.py.
I also think that we want the fix to be a bit deeper than createSource
. When creating an isochrone it should always respect the bands provided in the config regardless of whether the source section exists or not. I think this means we need to dig into what the factory is doing when it creates the isochrone.
oh, I think my hacked changes to MAGERR_PARAMS
were lost when I updated my fork, which is why I was confused to get that error again. I agree that createSource
is not the root of the issue, but that's also the earliest place I see a config file getting passed to the Source
object.
Will we want to look for a config file whenever we create an isochrone (i.e. in the factory)? or do we just want to load more parameters whenever we do read in a config file? I'm not sure how we would make this check whenever initializing an isochrone, which is what I think would need to happen at the factory level if we wanted to do that.
That's a good point. We would like to retain the ability to create a source without a config, which means that createSource
is probably the place we need to do it. However, we want to have the default action be to read the proper mag_1
and mag_2
variables without any source defined.
Since we'll have to read this from the config file, how would this be best implemented? I've been playing around with it a bit, and we could have createSource
look in the isochrone
section of the config file and then on source.isochrone
. However, since the source
section just appears to be a wrapper for isochrone
and kernel
, I'm not sure if this is much better than being ambivalent to the existence of a source
section...
As far as running the mcmc, this has been resolved by adding band_1
and band_2
specifications to srcmdl.yaml
in the isochrone
section. Right now, this line will load the isochrone specified by the srcmdl and overwrite the isochrone loaded from the config file. If we want to use non-default isochrone parameters, then these should be specified in both config.yaml
and srcmdl.yaml
for now.
Thanks Sid, this sounds like the underlying issue.
The mcmc.py
script allows (too much) flexibility in defining the source model and fit parameters. Looking in mcmc.py
I think the order of operations is:
source
variable in the mcmc
section of the configsrcmdl
file is provided, then overwrite with the values from the section of the srcmdl
file corresponding to the source name.params
variable exists in the mcmc
section of the config, then free the listed params in the source.opts.grid
is set, then initialize the parameter values to the best fit from an initial grid scan.Each of these pieces of functionality have been useful in the past, since they allow a lot of flexibility in the input parameters. For example:
Clearly, this complexity and lack of documentation has been detrimental for users. I think we can deal with this in a few ways.
srcmdl
in order to fit a candidate.
Working on MCMC analysis with ugali and tyring to use
g-i
bands has shown that ugali.analysis.source seems to create ag-r
isochrone even when the relevant config files specifyband_1 = g
andband_2 = i
. This behavior has been evidenced inugali/pipeline/run_05.0_followup.py
andugali/analysis/mcmc.py
, but may be elsewhere.