pyspeckit / paper1

First paper about pyspeckit
https://pyspeckit.github.io/paper1/main_compressed.pdf
1 stars 7 forks source link

JEP comments #10

Closed jpinedaf closed 6 years ago

jpinedaf commented 6 years ago

I added some comments, I am working on the N2H+ comparison, but I am having problems exporting the FITS file from pyspeckit into CLASS

keflavich commented 6 years ago

Anything I can help with on the FITS->CLASS front?

jpinedaf commented 6 years ago

I got it to work! ... so I am now adding the numbers in the table and description of comparisoin

keflavich commented 6 years ago

Thanks @jpinedaf, this is great! I merged your changes and addressed several of your comments.

I also added some text discussing the results from the CLASS<->pyspeckit comparison. The results are all nearly indistinguishable except that CLASS is less accurate in the velocity; my best guess is that they limit the parameter step size for velocity to be some reasonable subset (e.g., 1%) of the pixel size, so I stated that approximate speculation in the text. Do you think that's a good guess?

jpinedaf commented 6 years ago

I am not sure that is the reason for the velocity difference. In theory, CLASS is using the Minuit package for minimization, so that should not be a reason.

On the other hand, pyspeckit does not report an uncertainty to the centroid velocity... is this a bug?

keflavich commented 6 years ago

I don't know MINUIT well, but many optimization packages will set a minimum step size for certain parameters.

Yes, there is a big problem if pyspeckit isn't reporting an error on the centroid; I've never seen that happen. It might be that the error is somehow below the machine precision level...

jpinedaf commented 6 years ago

Could you run the generate_N2H+.py file to reproduce the behavior?

keflavich commented 6 years ago

Yes, I was able to reproduce the behavior. I don't understand it yet.

vlas-sokolov commented 6 years ago

Huh, the proposed changes in velocity centroid of the line (~1e-11) inside the fitter are below the tolerance level (1e-10). Also looks like zero is special somehow for tolerance calculations - works fine for bigger true x_offs.

if you want to list the full under-the-hood machinery of the fitters you need to set astropy logger to debug level (astropy.log.setLevel(10)) - the debug and verbose settings alone are not going to cut it...