Closed galaxyumi closed 10 months ago
Thanks @jacquesalice!! I really appreciate all your feedback. I made updates based on your suggestions/feedback. Please take a look at them and let me know what could be improved further.
A very nice NB @galaxyumi , thank you for writing it!
My comments below:
The NB is currently saved as executed with the 'ASTR285' kernel. Re-execute with the default 'Python 3' kernel, and save.
In TOC and section title: "... across the M31's disk" --> Drop "the"
in Summary: "Studying external galaxies, which are free of these projection effects, instead offers ..." --> "Studying external galaxies instead, which are free of these projection effects, offers ..."
In Summary: "... has various structures including spiral arms, star-forming rings, bar, and bulge." --> "... has various structures including spiral arms, star-forming rings, a bar, and a bulge."
In Summary: "Panchromatic Hubble Andromeda Treasury (PHAT; PI Dalcanton)" --> Can you link to a paper? (probably this one: https://ui.adsabs.harvard.edu/abs/2012ApJS..200...18D/abstract )
In Summary: "... covering from the ultraviolet through the near infrared." --> "... covering wavelengths from the ultraviolet through the near infrared."
In Disclaimers: I don't think we want to require that people reference this paper when they use our NB; better just have this paper in the "References" section. "PHAT Reduction paper: Williams et al., "Reducing and Analyzing the PHAT Survey with the Cloud", ApJS, 2018, 236, 4: https://ui.adsabs.harvard.edu/abs/2018ApJS..236....4W"
Remove , storeClient as sc
from the import cell, as it's not being used in this NB
In Section "Explore the main PHAT object table" link "M31withPhat.ipynb" to this URL: https://github.com/astro-datalab/notebooks-latest/blob/master/03_ScienceExamples/ExploringM31/M31WithPhat.ipynb
Propose to use this simpler query procedure (even if it's theoretically less robust; but that's a rare occurrence):
result = qc.query(sql=query) # by default the result is a CSV formatted string
instead of:
try:
result = qc.query(sql=query) # by default the result is a CSV formatted string
except Exception as e:
print(e.message)
df = qc.query(sql=query,fmt='pandas')
print("Number of columns:",len(df.columns))
print("List of columns:", df.columns)
df
Similar for other queries (keeps the NB shorter and simpler)
First time you introduce "MS stars", spell out "Main Sequence" maybe? Possibly also for "RGB stars".
At first it's not clear why we plot the first healpy figure that shows the brickID as function of RA and Dec. Later this makes it easier to refer to specific regions in M31, e.g. "brick 23 is the outermost region", etc. Maybe motivate the first plot in a sentence?
It seems that the norm=
argument to hp.gnomview()
has no effect (with any norm string given).
In the hp.gnomview()
figures, the max
parameter is empirical? Can it be programmatic? (i.e. determined at runtime from the data)
In the hp.gnomview()
figures, the colorbars need a label (e.g. "stars / healpixel", or (if converted properly) "stars / deg^2", etc.
For both the MS and RGB stars hp.gnomview()
figures, I suggest that the "conclusion" sentence be after the figure, not before. I.e., "Young MS stars are clearly clustered around the spiral arms and the 10 kpc ring, where active recent star formation takes place." and "Contrary to MS stars, old RGB stars show a smoother spatial distribution, mainly following a stellar density profile described as a combination of an exponential disk and a bulge."
Before each figure, it could just say "We are now plotting the distribution of MS stars in M31". etc.
Can the specific selection criteria / cuts in the queries for MS and RGB stars be easily motivated? (e.g., saying "using selection criteria from paper XYZ", or something equivalent).
In Section title "Do query for Brick 1 (most crowded centeral region)", fix typo "centeral" --> "central".
In Section "Do query for Brick 1 (most crowded centeral region)", the first query string doesn't need the parentheses grouping of the AND conditions (since they are all associative). I.e. you can replace
WHERE ((f275w_gst=1 AND f336w_gst=1) AND
(f475w_gst=1 AND f814w_gst=1) AND
(f110w_gst=1 AND f160w_gst=1)) AND
brick=1
with the simpler
WHERE f275w_gst=1 AND f336w_gst=1 AND
f475w_gst=1 AND f814w_gst=1 AND
f110w_gst=1 AND f160w_gst=1 AND
brick=1
Same for the query in Section "Do query for Brick 15 (10 kpc star-forming ring region)"
Same for the query in Section "Do query for Brick 23 (outer most low-density region)"
First time you use "CMD", spell out "Color Magnitude Diagram"
The new function make_cmds()
could still be simplified to reduce code repetition. I've streamlined it a bit and tested:
def make_cmds(brick,starlist=None,cmap='gray_r'):
"""
brick: Pandas dataframe for a given PHAT Brick
starlist: list of indices for stars (default=None)
"""
if starlist is None:
cmap = 'viridis'
fig, (ax1, ax2, ax3) = plt.subplots(1,3,figsize=(18,4))
def plot_panel(ax,x,y,range_=None,xlabel='',ylabel='',title='',starlist=None):
h = ax.hist2d(x,y,bins=200,range=range_,cmap=cmap,norm=plt.matplotlib.colors.LogNorm())
ax.set_xlabel(xlabel,fontsize=15)
ax.set_ylabel(ylabel,fontsize=15)
ax.set_xlim(h[1].min()-0.5,h[1].max()+0.5)
ax.set_ylim(h[2].max()+1,h[2].min()-1)
ax.set_title(title,fontsize=20)
if starlist is not None:
colors = cycle(mcolors.TABLEAU_COLORS) # recurring cycle of defined plot colors
ax.scatter(x[starlist], y[starlist], s=50, c=[next(colors) for j in range(len(starlist))])
plot_panel(ax1,brick['f275w_vega']-brick['f336w_vega'],brick['f336w_vega'],((-2,4),(15,27)),'F275W - F336W','F336W','UV CMD',starlist)
plot_panel(ax2,brick['f475w_vega']-brick['f814w_vega'],brick['f814w_vega'],((-1,6),(14,29)),'F475W - F814W','F814W','Optical CMD',starlist)
plot_panel(ax3,brick['f110w_vega']-brick['f160w_vega'],brick['f160w_vega'],((-1,3),(11,28)),'F110W - F160W','F160W','IR CMD',starlist)
plt.show()
Note that I've used cycle
from itertools and colors
from matplotlib, to get the same colors here and later in the function make_seds
. This requires two extra lines in the Imports cell:
In the # std lib
section:
from itertools import cycle
and just after import matplotlib.pyplot as plt
:
import matplotlib.colors as mcolors
make_seds()
slightly (to get consistent plotting colors):def make_seds(brick,k=3):
fig, ax = plt.subplots(1, 1, figsize=(6,4))
stars_6b = good_stars(brick)
sIDs = random.choices(stars_6b, k=k)
c = cycle(mcolors.TABLEAU_COLORS) # recurring cycle of defined plot colors
for j,i in enumerate(sIDs):
ax.plot(plambda, [brick['f275w_vega'][i], brick['f336w_vega'][i],
brick['f475w_vega'][i], brick['f814w_vega'][i],
brick['f110w_vega'][i], brick['f160w_vega'][i]], marker='.', ls='-', c=next(c))
ymin = np.min(brick.iloc[sIDs].values.ravel())
ymax = np.max(brick.iloc[sIDs].values.ravel())
ax.set_ylim(ymax+0.5, ymin-0.5)
ax.set_xlabel(r'$\lambda$ [nm]',fontsize=15)
ax.set_ylabel('magnitude',fontsize=15)
make_cmds(brick, sIDs)
plt.show()
Note also that I made 'k' an argument to make_seds()
so that users can specify how many random stars they want to get.
make_seds(df_b1,k=3)
, for instance.
"Brick 15 covers a portion of the 10 kpc star-forming ring of the M31's disk."
to
"Brick 15 covers a portion of the 10 kpc star-forming ring in M31's disk."
@rnikutta Thanks for the comments. I implemented all of your comments and also added an html version. I think this NB is ready to be merged, and could be included in the newsletter if you want!
Thank you @galaxyumi , NB looks very good! Could you please make one more commit with these tiny changes, before I merge?
from dl.helpers.utils import convert
hp.gnomview()
plots of MS and RGB stars. This probably isn't robust if someone were to change the query constraints, or changed the healpix resolution... OK to keep as-is for now, but maybe consider generalizing it in a future PR?
Thank you!@rnikutta All 4 points were addressed.
@galaxyumi Sorry to bother you again, but I noticed that your NB is saved without the outputs. We usually store them with the outputs, unless the outputs make the ipynb file very large (multi-MB). Could you please re-run, save with the outputs, and also generate the HTML preview with the outputs?
This will also retain the per-cell-runtime lines (which we want).
Many thanks!
Done! Thank you.
Thank you @galaxyumi for the NB, merging now!
The NB draft is ready for review. With some confusion in ZP values with photometry, I decided to plot stellar SEDs in magnitude in stead of converting them into rates or fluxes. This could be replaced later when the rate columns in the PHAT tables are properly re-ingested.