-
Notifications
You must be signed in to change notification settings - Fork 38
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
ENH: example for galaxy demographics #520
base: main
Are you sure you want to change the base?
Conversation
@rrjbca to play the audio files, I'll use this Does that mean we need to make IPython a dependency in SkyPy only to run the example? |
Sorry I only just saw this. Yes you should just add |
|
Check the logs, it's a kstest failure, just re-run it |
The docs are not picking up the latest changes but it renders well on my local machine. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @Lucia-Fonseca, the example looks really nice. Please find some comments below.
One thing beside the comments is that it might be good to increase the tick labels. They seem a bit small.
|
||
for ax in [ax1, ax2, ax3]: | ||
ax.legend(loc='lower left', fontsize='small', frameon=False) | ||
ax.set_xlabel(r'Stellar mass, $log (M/M_{\odot})$', fontsize=18) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ax.set_xlabel(r'Stellar mass, $log (M/M_{\odot})$', fontsize=18) | |
ax.set_xlabel(r'Stellar mass, $\log (M/M_{\odot})$', fontsize=18) |
This is to prevent log
from being in italics.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe increase the fontsize for both labels
ax.set_yscale('log') | ||
|
||
|
||
ax1.set_ylabel(r'$\phi /h^3 Mpc^{-3}$', fontsize=18) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ax1.set_ylabel(r'$\phi /h^3 Mpc^{-3}$', fontsize=18) | |
ax1.set_ylabel(r'$\phi /h^3$\,Mpc$^{-3}$', fontsize=18) |
I prefer and learned to not put the units into math mode. This makes it easier to distinguish between parameters/variables and units.
z_centrals, m_centrals = schechter_smf(z_range, mstarb, phic, alphab, 1e9, 1e12, sky_area, cosmology) | ||
z_satellites, m_satellites = schechter_smf(z_range, mstarb, phis, alphab, 1e9, 1e12, sky_area, cosmology) | ||
z_massq, m_mass_quenched = schechter_smf(z_range, mstarb, phimq, alphab + 1, 1e9, 1e12, sky_area, cosmology) | ||
z_satq, m_satellite_quenched = schechter_smf(z_range, mstarb, phisq, alphab, 1e9, 1e12, sky_area, cosmology) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
z_centrals, m_centrals = schechter_smf(z_range, mstarb, phic, alphab, 1e9, 1e12, sky_area, cosmology) | |
z_satellites, m_satellites = schechter_smf(z_range, mstarb, phis, alphab, 1e9, 1e12, sky_area, cosmology) | |
z_massq, m_mass_quenched = schechter_smf(z_range, mstarb, phimq, alphab + 1, 1e9, 1e12, sky_area, cosmology) | |
z_satq, m_satellite_quenched = schechter_smf(z_range, mstarb, phisq, alphab, 1e9, 1e12, sky_area, cosmology) | |
z_centrals, m_centrals = schechter_smf(z_range, mstarb, phic, alphab, | |
1e9, 1e12, sky_area, cosmology) | |
z_satellites, m_satellites = schechter_smf(z_range, mstarb, phis, alphab, | |
1e9, 1e12, sky_area, cosmology) | |
z_massq, m_mass_quenched = schechter_smf(z_range, mstarb, phimq, alphab + 1, | |
1e9, 1e12, sky_area, cosmology) | |
z_satq, m_satellite_quenched = schechter_smf(z_range, mstarb, phisq, alphab, | |
1e9, 1e12, sky_area, cosmology) |
Adding line breaks prevents the user from scrolling horizontally in the docs.
# from skypy.galaxies.stellar_mass import (schechter_smf_amplitude_centrals, | ||
# schechter_smf_amplitude_satellites, | ||
# schechter_smf_amplitude_mass_quenched, | ||
# schechter_smf_amplitude_satellite_quenched | ||
# ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand this can only be included after #516 is merged. Please adapt the indentation when when this PR is ready to be merged.
# Schechter Parameters | ||
# -------------------- | ||
# | ||
# We generate their Figure 4 and use the best-fit values of the blue parameters |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# We generate their Figure 4 and use the best-fit values of the blue parameters | |
# We generate Figure 3 in de la Bella et al. 2021 and use the best-fit values of the blue parameters |
fig, (ax1, ax2, ax3) = plt.subplots(nrows=1, ncols=3, figsize=(16, 6), sharex=True, sharey=True) | ||
fig.suptitle('Galaxy Demographics Simulation', fontsize=26) | ||
|
||
ax1.plot(wblue['logm'], 10**wblue['logphi'], color='k', label='Weigel+16 active', lw=1) | ||
ax1.step(bins[:-1], logphi_active, where='post', label='SkyPy active', color='blue', zorder=3, lw=1) | ||
ax1.step(bins[:-1], logphi_centrals, where='post', label='SkyPy centrals', color='royalblue', zorder=3, lw=1) | ||
ax1.step(bins[:-1], logphi_satellites, where='post', label='SkyPy satellites', color='cyan', zorder=3, lw=1) | ||
|
||
|
||
ax2.plot(wred['logm'], 10**wred['logphi'], color='k', label='Weigel+16 passive', lw=1) | ||
ax2.fill_between(wred['logm'], 10**wred['upper_error'], 10**wred['lower_error'], color='salmon', alpha=0.1) | ||
ax2.step(bins[:-1], logphi_passive, where='post', label='SkyPy passive', color='red', zorder=3, lw=1) | ||
ax2.step(bins[:-1], logphi_massq, where='post', label='SkyPy mass-quenched', color='coral', zorder=3, lw=1) | ||
ax2.step(bins[:-1], logphi_satq, where='post', label='SkyPy sat-quenched', color='maroon', zorder=3, lw=1) | ||
|
||
ax3.plot(wtotal['logm'], 10**wtotal['logphi'], color='k', label='Weigel+16 total', lw=1) | ||
ax3.plot(wcentral['logm'], 10**wcentral['logphi'], '--', color='grey', label='Weigel+16 centrals', lw=1) | ||
ax3.plot(wsatellite['logm'], 10**wsatellite['logphi'], '--', color='grey', label='Weigel+16 satellites', lw=1) | ||
ax3.fill_between(wtotal['logm'], 10**wtotal['upper_error'], 10**wtotal['lower_error'], color='plum', alpha=0.1) | ||
ax3.step(bins[:-1], logphi_total, where='post', label='SkyPy total', color='purple', zorder=3, lw=1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add line breaks here such that the user does not need to scroll horizontally?
ax3.plot(wcentral['logm'], 10**wcentral['logphi'], '--', color='grey', label='Weigel+16 centrals', lw=1) | ||
ax3.plot(wsatellite['logm'], 10**wsatellite['logphi'], '--', color='grey', label='Weigel+16 satellites', lw=1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it make sense to plot these two lines with the same line style?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What are these lines supposed to demonstrate in the first place? Does it make sense to remove them or also show the SkyPy fits of these two components?
fig, (ax1, ax2, ax3) = plt.subplots(nrows=1, ncols=3, figsize=(16, 6), sharex=True, sharey=True) | ||
fig.suptitle('Galaxy Demographics Simulation', fontsize=26) | ||
|
||
ax1.plot(wblue['logm'], 10**wblue['logphi'], color='k', label='Weigel+16 active', lw=1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Weigel+ lines for the passive and the total sample include errors. Does the blue sample not have errors?
|
||
|
||
ax1.set_ylabel(r'$\phi /h^3 Mpc^{-3}$', fontsize=18) | ||
# plt.savefig('galaxy_simulation.pdf') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this comment
# is becoming increasingly important to make astronomy accessible | ||
# for those who are visually impaired, and to enhance visualisations | ||
# and convey information that visualisation alone cannot. | ||
# In this work [1] the authors also made their main plot available |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# In this work [1] the authors also made their main plot available | |
# In [1]_, the authors also made their main plot available |
Description
This PR adds an example to reproduce Figure 3 in de la Bella et al. 2021. Include sonification feature from Strauss. Merging this PR closes #514 .
Inputs:
References
Checklist