jotaylor / spamm Goto Github PK

With dill version 0.2.8.2, the function dill.dill was replaced with dill._dill. Even though there is a supposed fix for this it's not working for me when unpickling pickle files made with 0.2.7.1. The only solution is to downgrade dill to 0.2.7.1 to load older pickle files.

Shouldn't be an issue for pickle files made with 0.2.8.2.

This is mostly an issue for me during testing.

Issue transferred from justincely/spamm:
Create emission lines components (broad and narrow)

Specifically, we saw some problems when testing broken power laws.

Now that we have legit docs, it's time to remove the wiki.

Let's use something with more components for the example case.

The refactored specutils version uses Tabular1D in wcs_wrapper:
https://github.com/astropy/specutils/blob/26d80182b10a62f064caa619363d520cdc944e66/specutils/wcs/wcs_wrapper.py#L58
But Tabular1D can't be pickled

> this = Tabular1D(np.arange(10), np.arange(10))                                                                                                             

> pickle.dump(this, open("tester.p", "wb"))                                                                                                                  
---------------------------------------------------------------------------
PicklingError                             Traceback (most recent call last)
<ipython-input-21-fd38e6f8bdb3> in <module>
----> 1 pickle.dump(this, open("tester.p", "wb"))

PicklingError: Can't pickle <class 'abc.Tabular1D'>
Name: Tabular1D
Inputs: ('x0',)
Outputs: ('y',): attribute lookup Tabular1D on abc failed

This problem propagates all the way through so that when a Model instance is returned by SPAMM, it cannot be pickled. This is a really big deal, since there's a ton of information stored in Model and the emcee.sampler instance. I tried saving Model as a json, but nothing worked.

Issue transferred from justincely/spamm:
Test it or kill it

It's a bit misleading because specutils.Spectrum1D uses uncertainty while spamm.Spectrum uses flux_error.

We can copy the strategy used in pysynphot.observation.initbinflux:
https://github.com/spacetelescope/pysynphot/blob/master/pysynphot/observation.py#L328

They're scattered everywhere right now. I think the answer is to more plot_spamm_results.py to spamm/ or Analysis.py or something. ???

Issue transferred from justincely/spamm:
Right now, they output to screen and it is not obvious they should be closed to continue script.

https://github.com/antking/SPAMM/blob/907abbe494c7fd45fc256c50712517c9943fc93c/spamm/components/HostGalaxyComponent.py#L322

In the WorkingHostGalaxyandProperlyRedshift branch another prior is added after normalization priors. Is this a bug?

Issue transferred from justincely/spamm:
Test/Improve Anthea's implementation of Balmer Continuum/pseudo continuum

Anthea: Edited psuedo-continuum algorithm so that electron density is included properly. There was a typo previously. Also have created a new function for estimating the intensities of 51<n<400 Balmer lines, which estimates the excitation temperature from the Storey & Hummer ratios. The fit is still not great to the fake data, this is partly because I don't know what electron density was used in making the fake data and whether we need to fit for this. My method of estimating the excitation temperature may also be inaccurate, but the reference paper Kovacevic et al 2014 (http://www.sciencedirect.com.ezp.lib.unimelb.edu.au/science/article/pii/S0273117713007382?np=y) was very unclear about this.

I've started it, but need to finish it.

Eventually the flux binning should be a part of Spectrum:
https://github.com/antking/SPAMM/blob/cleanupcode/utils/rebin_spec.py

However, it would be nice if it were a part of the underlying specutils Spectrum1D object. Once it gets implemented there, we'll inherit the method.

https://github.com/antking/SPAMM/blob/9895fc1379275ee630036af08bb10ae445df14c8/spamm/Spectrum.py#L28

@antking I believe you added the mask sections to Spectrum. I'm trying to clean up/document code and can't figure out where the attribute self.maskFWHM_broad gets mask_FWHM_broad from. It isn't defined in Spectrum or supplied through the init. Any help?

I don't think this works right now.

Issue transferred from justincely/spamm:
gisederosa:
Improve performance of stellar dispersion modeling

Roberto: I have implemented the stellar dispersion parameter in the host templates, according to our last discussion. It is extremely slow and it is not activated so as to not cause delays on the tests of other components. To activate it, you need to comment and uncomment the lines that calculate the Kmat matrix in the flux subroutine of the Host Galaxy component.

antking:
I have changed how stellar dispersion is implemented in the host galaxy component that runs significantly faster but it needs more testing. I cannot find where the true values are for the fake host component data are. Can someone point me in the right direction and I will perform the necessary tests?

https://github.com/antking/SPAMM/blob/758bb3247bead8f288b269f113baec7a28c8f990/examples/simple_run_general.py#L156

With pickle or cPickle, pickle dump crashes with:
*** _pickle.PicklingError: Can't pickle <class 'weakref'>: attribute lookup weakref on builtins failed
The only workaround I've found is to use dill:
https://pypi.python.org/pypi/dill

However, this creates an additional package dependency for the user. We should investigate the root issue and try to fix it eventually.

Issue transferred from justincely/spamm:
Implement broken power-law + comptonized disk

Parallel processing needs to be tested and optimized.

Since the normalizations are linked to the order of the templates, we need to ensure they are always loaded in the order the user intends to. A reasonable approach to this could be ordering the templates by starting wavelength.

However, while this will work for the iron component, it will not for host galaxy. Maybe a more straightforward alphabetization would be best.

like there are so many things being carried around... maybe it needs to all be encapsulated into a class?

I think we only use BalmerContinuumCombined.py? @antking can I delete BalmerContinuum.py, BalmerContinuum_AK_super.py, and BalmerPseudoContinuum.py?

I would then rename BalmerContinuumCombined.py to BalmerContinuum.py.

Right now we use v 0.2.2 and should upgrade to 0.3. The updates are significant enough that it may require some non-trivial amount of work.

Issue transferred from justincely/spamm:
Push latest updates

We need to move the repo to either myself or @gisederosa 's personal account, then that person will transfer it to STScI's account.

Right now I'm using dummy errors and I'm 99.9% sure this is wrong...........

It could be cleaned up a bit, including

• having fewer iteration printouts
• getting rid of some useless text

Currently it looks like:

Mean acceptance fraction: 0.332
Saved pickle file 20190522_1327/model_20190522_1327.pickle.gz
	Saved model_20190522_1327_triangle.png
Plotting the MCMC chains.
	Saved model_20190522_1327_chain.png
Plotting the model posterior PDFs.
	Saved model_20190522_1327_posterior.png
Saved 20190522_1327/model_20190522_1327_posterior.pdf
executed in 0:00:12.307435

Currently they are stored in a list, which requires params to be in a specific order and accessed through self.parameter.index. This is a very error prone method. Instead, we should think about changing the parameters to be stored in a dictionary.

parse_pars.py needs to be updated, it is throwing the following warning:

parse_pars.py:22: YAMLLoadWarning: calling yaml.load() without Loader=... is deprecated, as the default Loader is unsafe. Please read https://msg.pyyaml.org/load for full details.
  pars = yaml.load(f)

Right now it's only accessible through Samples. Maybe through #34 ?

Sometimes this error occurs when using statistics.mode. Re-running SPAMM once or twice gets rid of it.

Mean acceptance fraction: 0.358
Saved pickle file 20190523_5826/model_20190523_5826.pickle.gz
Traceback (most recent call last):
  File "test_spamm.py", line 138, in <module>
    test_spamm(components=comps, n_walkers=int(args.n_walkers), n_iterations=int(args.n_iterations))
  File "test_spamm.py", line 107, in test_spamm
    n_iterations=n_iterations)
  File "/user/jotaylor/git/spamm/spamm/run_spamm.py", line 156, in spamm
    make_plots_from_pickle(pname, outdir)
  File "/user/jotaylor/git/spamm/spamm/analysis.py", line 245, in make_plots_from_pickle
    S = Samples(pname, outdir=outdir, gif=gif, last=last, step=step)
  File "/user/jotaylor/git/spamm/spamm/Samples.py", line 31, in __init__
    self._get_stats()
  File "/user/jotaylor/git/spamm/spamm/Samples.py", line 55, in _get_stats
    self.modes.append(statistics.mode(chain))
  File "/Users/jotaylor/anaconda3/envs/spamm/lib/python3.7/statistics.py", line 506, in mode
    'no unique mode; found %d equally common values' % len(table)
statistics.StatisticsError: no unique mode; found 2 equally common values

Right now they are interpreted literally as strings, e.g. 1.6e-3 is read in as "1.6e-3" and not .00167.

There is an rc version of emcee available. The practical changes are small, mostly in the API.
https://github.com/dfm/emcee/releases/tag/v3.0rc2

Finalize YAML input work. I have created a preliminary yaml file but have no way to read in yet.

Right now it is set to np.random.uniform by default. We can add this in the yaml input.

I did some line profiling today and found that the numpy convolve function is several factors faster than the currently implemented pyfftw covolution function. I have replaced the function in the local balmercontinuumcombined log_conv function but should be performed throughout (aka in host galaxy and Fe template and eventually in spectrum object or utils function)

replacement should be done as followed
flux_conv = fftwconvolve_1d(flux_rebin, kernel)
to
flux_conv = np.convolve(flux_rebin, kernel,mode='same')