annayqho / TheCannon

a data-driven method for determining stellar parameters and abundances from stellar spectra
MIT License
41 stars 17 forks source link

added parallel computing for continuum normalization process #68

Closed hypergravity closed 8 years ago

hypergravity commented 8 years ago

Since it is extremely slow when normalizing a huge number of spectra using only 1 process, I added parallel computing for continuum normalization process, using the multiprocessing module. Because the multiprocessing takes more memory than single process, you can specify the keyword arg n_proc to choose the number of processes to be used. Another keyword arg added by me is verbose, which determines whether it will print information during the continuum normalization or not.

Example, simply use these two lines your example instead:

pseudo_tr_flux, pseudo_tr_ivar = ds.continuum_normalize_training_q(
    q=0.90, delta_lambda=100, n_proc=n_proc, verbose=True)

and

cont = ds.fit_continuum(3, "sinusoid", n_proc=n_proc)

I have tested the modified version of TheCannon, it seems that all things go correctly, except that I sometimes got a Warning from the curve_fit function, saying that the covariance of parameters could not be estimated. Then I checked your code and found that actually the cov of the parameters of the continuum is not used in the subsequent data processing, and also the results seems to be the same as the original version TheCannon, so I just ignore these warnings.

I hope you can spend some time to check the lines inserted and guarantee that my understanding is right.

By the way, I checked the training process but found no related code dealing with the input model.TheCannon.order, which means that in this version the order is specified to be 2 and if I change

md = model.CannonModel(2)

to

md = model.CannonModel(3)

it makes no difference, am I right?

Cheers, Bo Zhang bozhang@nao.cas.cn

annayqho commented 8 years ago

Sorry for the slow response! Thank you for including the parallel computing, looks great. And yes, you're right, currently the order of the model isn't being used, although it would be straightforward to add this in (building a polynomial of arbitrary order, with the corresponding number of coefficients). That's been on my to-do list for a while but I haven't gotten around to it yet...

hypergravity commented 8 years ago

Thanks for your merge and reply. I hope I can add a flexible model to this package sometime.

Cheers, Bo Zhang