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

PPP catalog #147

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

PPP catalog #147

wants to merge 2 commits into from

Conversation

dcolombo
Copy link

@dcolombo dcolombo commented Feb 7, 2016

Ana Duarte-Cabral and I created a tentative version of PPP structure catalog.

The current implementation works only on the volumetric density-based simulated data cube used to generate the dendrogram. Simulated data cubes must be provided in units of g/cm3 or Msun/pc3. Classes needs mass.py script functions which provide the mass of the structures in Msun. This is basically a mirror of flux.py of PPV catalog.

The implementation has been tested on a chunk of Clare Dobbs' simulation and appears to provide the expected results.

@keflavich
Copy link
Contributor

On the failures: there seems to be a travis configuration issue, which I'm attempting to fix here: #148

pixel_volume = ((spatial_scale.to(u.cm)) ** 3)

elif spatial_scale.unit.is_equivalent(u.pc):
pixel_volume = ((spatial_scale.to(u.cm)) ** 3)
Copy link
Contributor

Choose a reason for hiding this comment

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

this & the next line are redundant: u.cm, u.pc, and u.kpc are all equivalent

@keflavich
Copy link
Contributor

Besides the few comments, this looks good to me. It would be helpful to have some tests, though.

@smhr
Copy link

smhr commented Feb 18, 2021

Hello @keflavich and @dcolombo. I want to use PPP statistics to find the mass of structures (leaves) in a simulation outputs. I think this PR is very useful. But I see this PR has remained open since a long time. Could I use the fork safely? Has anyone tested it?
Regards

@pscicluna
Copy link

Hi! I'm also interested in using this, so a status update would be handy. Are there things that I could do to help move this forward? Or should I be looking at re-writing the functionality that I need myself?

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.

4 participants