phangsteam / pycprops Goto Github PK

Distance determination should be a constant or accept coordinates as a function to return distances (for, e.g., kinematic distances).

It looks like commit 6deac16 introduced a syntax error. There is a missing parenthesis in cloudalyze.py on line 101.

It needs to be switched to

    if (len(x[good][slc]) == 0) or (np.all(x[good][slc]) == 0):
                                                       ^
                                                       |

Let me know if you need any more information.

I think there are some errors in the way uncertainties are calculated on the cloud properties. I'm sorry I don't have more time to check these right now, but here is what I found. It's roughly ordered from what I'm more confident is incorrect to less. These are all in cloudalyze.py.

1. RAD_UC, RAD_NODC_UC, RAD_NOEX_UC, and RAD_NODC_NOEX_UC combine rmsx_* with itself in quadrature instead of rmsx_* with rmsy_*.
2. The uncertainty on the virial mass (line 431) should multiply the velocity dispersion uncertainty by 4, not 2. thiscloud["MVIR_UC"] = np.sqrt(thiscloud["RAD_UC"] ** 2 + 4 * thiscloud["SIGV_UC"] ** 2)
3. Should the uncertainties on the deconvolved quantities be multiplied by an extra factor from the deconvolution compared to the non-deconvolved versions?
   • E.g. the relative uncertainty on SIGV_KMS would be ((moments_rot['rmsv_ex'] * dv ** 2) / (moments_rot['rmsv_ex'] ** 2 * dv ** 2 - sigchan_kms ** 2)) * uc_dict['rmsv_ex'].
4. I think ucval and ucval_rot (lines 398 and 401) are supposed to be the relative standard error on the median. If that's right then here are a couple points.
   • Shouldn't the number of pixels per beam be replaced with the number of bootstrap samples?
   • Also, I think for the standard error on the median it should be np.sqrt(np.pi / 2) * (1 / np.sqrt(N + 2)) * (std / median) (from the approximation here, but could probably be improved with equation 2 here).

The Astropy ProgressBar in the cloudalyze function works as the loop is running, but it does not finish by reporting the total runtime for the loop and adding a newline. This seems to be because the ProgressBar object is not closed/released properly at the end of the loop. Instead of line 212 and 213 which are

if verbose:
    bar = ProgressBar(len(uniqlabels))

the entire loop block (down to and including the current line 408) should be within a with statement like

    if verbose:
        barfile = None
    else:
        barfile = open(os.devnull, 'w')
    with ProgressBar(len(uniqlabels), file=barfile) as bar:
        for cloudnum, thislabel in enumerate(uniqlabels):
#
# ...snip...
#
            cloudlist += [thiscloud]
            if verbose:
                bar.update()
            else:
                barfile.close()
    outtable = Table(cloudlist)
    return(outtable)

When the ProgressBar file is None then it sends its output to the terminal. Otherwise we just send the output to the void. I think this also sets some of the groundwork for saving log files which could include the runtime of these steps by swapping out os.devnull for the log file.

When the message Calculating cloud properties for # clouds appears in the terminal it it usually one cloud more than in the resulting tables of cloud properties. It is also wrong in the progress bar that follows that message.

I think it has to do with skipping labels that are zero (line 224 in cloudalyze.py) since the number in the message is calculated previously on all unique labels, potentially including zero (line 219).

Currently the README gives the following example for running fits2props

import pycprops
import astropy.units as u
cubefile = 'mycube.fits'  # Your cube
mask = 'mycube.mask.fits' # Mask defining where to find emission
d = 8.0 * u.kpc           # Distance (with units)

pycprops.fits2props(cubefile,
                    mask_file=mask,
                    distance=d,               
                    asgnname='mycube.asgn.fits',
                    propsname='mycube.props.fits')

but since the default value of the noise_file argument is None this example crashes with

File "/cavern/home/brunettn/bin/miniconda3/envs/research/lib/python3.7/site-packages/pycprops-0.1.dev20+g52aad10-py3.7.egg/pycprops/pycprops.py", line 40, in fits2props
    noise = SpectralCube.read(datadir + '/' +noise_file)
TypeError: can only concatenate str (not "NoneType") to str

The example should be updated to show all of the mandatory arguments, but fits2props should also be changed to give a more descriptive error about what is actually wrong (maybe there shouldn't be a default value for that and other arguments).

It looks like it should be passed to alllocmax but it is hard coded to 1 in that function call.

I tried running in Python 3.10.2 but encountered the following import error (partial traceback to exclude my scripts and directory names):

File ".../astrodendro/__init__.py", line 3
    from .dendrogram import Dendrogram, periodic_neighbours
File ".../astrodendro/dendrogram.py", line 8
    from collections import Iterable
ImportError: cannot import name 'Iterable' from 'collections'

I opened an issue in the dendrograms/astrodendro repo about this, but for pycprops it looks like you must use Python versions <= 3.9 until that issue is fixed in astrodendro.

Let me know if you need any more information.

I found that if I passed a FITS cube for the mask that had NaN pixels where I did not want the algorithms to work and float values where I did want it to work, then all pixels were included in processing. I believe it had to do with this line in pycprops.py

s = s.with_mask(mask.astype(np.bool))

where casting np.nan to bool results in True. I'm not sure if it's a Numpy version issue, but I can give some more info on my environment later if it would help. Sorry this is a bit terse, I wanted to bring this to your attention before I forgot (and I'm taking some time off the rest of this week).

Hi,

I was just wondering if there is a tweakable parameter built in that controls how emission is split up and assigned to individual clouds. At the moment the identified clouds output by the program are quite large, and I'm interested in comparing the output cloud parameters at different size scales.

thanks!

fix appears to be to modify _astropy_init.py - see radio-astro-tools/spectral-cube#891

Hi,

I'd very much like to use the pycprops package for my ALMA data and am looking for a few tips as its crashing currently. What are the desired units and format of the input cube? Should the polarization axis be trimmed? Does it need to be frequency, or velocity, and does it matter if it's Jansky or Kelvin? Furthermore I'm currently getting a crash saying my cube is too large (3 Gb) to load into memory, strangely this seems to be resolved if I specify a noise cube. What are the preferred units for the noise cube?

Thanks!