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

spectral-cube and CASA operations: performance comparisons #11

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

e-koch
Copy link
Collaborator

@e-koch e-koch commented Sep 9, 2020

Tutorial and overview of comparing dask and CASA performance.

Binder

@e-koch
Copy link
Collaborator Author

e-koch commented Sep 10, 2020

Bokeh plots are not being rendered on binder.

@e-koch e-koch changed the title First look through dask performance tutorial Tutorials linking spectral-cube and CASA operations: performance and user's translation guide Sep 25, 2020
@e-koch
Copy link
Collaborator Author

e-koch commented Sep 25, 2020

@astrofrog @keflavich I've added a CASA to spectral-cube conversion tutorial for basic cube operations and fitting:

Moved to #15

Everything won't work until radio-astro-tools/spectral-cube#656, radio-astro-tools/spectral-cube#665, and radio-astro-tools/spectral-cube#666 are merged in spectral-cube.

The performance tutorial above is still a WIP.

@e-koch
Copy link
Collaborator Author

e-koch commented Sep 25, 2020

This is also noted in the tutorial and demonstrated, but I think this could be a common discrepancy users may find so I'll retiterate it here:

The biggest spectral-cube to CASA difference is CASA's definition of the second moment along the spectral dimension. spectral-cube uses the correct formal definition (i.e., the variance) and then defines the sqrt as the "linewidth_sigma". If noisy lines of sight are included, some of the moment 2 values can be negative (because the variance is being weighted by negative values in places) and so the square root ends up being a NaN. This is likely the most "fair" way of handling these cases as a negative moment 2 is definitely dominated by noise features and not the actual signal. To correct for this, signal masking should be applied before computing the moment 2.

In CASA, the moment2 from immoments is spectral-cube's "linewidth_sigma" (https://casa.nrao.edu/casadocs/casa-6.1.0/global-task-list/task_immoments/about). However, CASA will also apply an absolute value before taking the square root. This will lead to overestimates of the line width and further hide where some lines-of-sight have an erroneous negative moment 2 value. In the CASA-to-SC conversion tutorial, I show that the line widths are overestimated by 5 times in the tutorial data when using this method versus first applying an appropriate signal mask.

If you need to reproduce the CASA behaviour for the second moment in spectral-cube, use this:

moment2 = cube.moment2()
moment2_quant = moment2.quantity
moment2_quant = np.abs(moment2_quant)
lwidth_abs = np.sqrt(moment2_quant)

Copy link
Member

@astrofrog astrofrog left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you remove the output cells from the notebooks? (otherwise one of the notebooks is too large to comment on - and anyway we should not store outputs in the repo)

"\n",
"## Summary ##\n",
"\n",
"This tutorial compares the performance of common operations in spectral-cube and CASA. This provides an idea for which operaitons in the respective packages are well optimized. The goal is also to explore how to produce equivalent comparisons since the package use different methods to parallelize operations.\n",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: operations

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also I'm not sure about calling this a tutorial, maybe just referring to it as a 'notebook' would make more sense? (and linking to the other notebook in this PR for the average user)

"pip install dask\n",
"pip install aplpy\n",
"pip install --index-url https://casa-pip.nrao.edu/repository/pypi-casa-release/simple casatools\n",
"pip install --index-url https://casa-pip.nrao.edu/repository/pypi-casa-release/simple casatasks\n",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put casatools and casatasks on same line, and also simplify imports above by removing ones that are redundant?

@e-koch e-koch changed the title Tutorials linking spectral-cube and CASA operations: performance and user's translation guide spectral-cube and CASA operations: performance comparisons Oct 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants