Closed JasonWooo closed 1 year ago
Thanks @JasonWooo!
As context for others: this submission is part of the final project for ASTR285 at UChicago. I plan to review the notebook before requesting review from a repo admin.
@JasonWooo I think this notebook would be ready for Data Lab review with some fairly minor changes. Let me know if you'd like to go forward with that process. I can leave some suggestions here and/or make some changes directly to the notebook.
Sure! I will be traveling tomorrow but I'll let you know once I'm settled down to work on things!
@jacquesalice and @rnikutta would you have a chance to give this notebook a review? Thanks!
Hi @kadrlica and @JasonWooo , gladly! Thanks for the NB. I'm still on vacation but will try to take it for a spin on Thu or Fri.
Really nice NB @JasonWooo and @kadrlica , many thanks! Below my comments in addition to @jacquesalice 's, trying not to repeat any:
In Summary section: "using data from the The Baryon Oscillation Spectroscopic Survey (BOSS)" --> "using data from The Baryon Oscillation Spectroscopic Survey (BOSS)"
"released a part of the SDSS DR12" --> "released as part of SDSS DR12"
If there is a good public link to "ASTR285 Science with Large Astronomical Surveys", you could link it... (in Summary section)
In "Early Universe": "The early universe is so hot and dense that matter existed in the form of plasma" --> "was so hot"
"Thompson scattering" --> "Thomson scattering"
Remove unused imports:
plt
, you can replace the single mpl
use by: plt.rcParams.update(mpl.rcParamsDefault)
"Loading Galaxy Data" section: We have the SDSS DR12 files at Data Lab, in the sdss_dr12://
file service. Using the file service to fetch the files will make a stronger connection of the NB to DL, and it simplifies the NB. I propose that you:
from dl import storeClient as sc
Loading Galaxy Data
Load the CMASS data sample from the Baryon Oscillation Spectroscopic Survey (BOSS). Only the Southern half of the survey is used. The analysis could extend to the full dataset, but this is sufficient for the purposes of seeing the BAO. The dataset is pre-selected for Luminous Red Galaxies (LRGs) using two different color selections for low-redshift (𝑧<0.45) and high-redshift (𝑧>0.45) galaxies. The specifics are noted in this SDSS page.
We get the FITS file from the sdss_dr12
File Service at Data Lab:
remdir = 'sdss_dr12://boss/lss/' # remote
locdir = './data/' # local
fname = 'galaxy_DR12v5_CMASSLOWZTOT_South.fits.gz'
if not os.path.exists(locdir+fname):
res = sc.get(fr=remdir+fname,to=locdir+fname,verbose=False)
Load the data and print the fields and number of objects in the dataset.
data = fits.open(locdir+fname)[1].data
print(data.dtype)
print(f'{len(data)} objects in data set')
Same for the "Random Data" section. The file is on our sdss_dr2://
file service. You can replace the wget cell with:
fname = 'random0_DR12v5_CMASSLOWZTOT_South.fits.gz'
if not os.path.exists(locdir+fname):
res = sc.get(fr=remdir+fname,to=locdir+fname,verbose=False)
and the data loading cell with:
rand = fits.open(locdir+fname)[1].data
print(rand.dtype)
print(f'{len(rand)} points in total in random')
"The random dataset would, however, be hard to construct in this case, as the SDSS random does not come with magnitudes assigned." --> Explain here briefly what is "SDSS random".
First two-panel plot in Section "Visualization of the Dataset": Alice already asked for a colorbar with units for the right-hand plot. Please also consider replacing the panel titles, from "Scatter" to "Distribution in the sky", and from "Density" to "Object density".
Same for the Scatter and Density plot in Section "Loading Random Data".
"Show a mollwide visualization" --> "Show a Mollweide visualization"
In the Mollweide view, the colorbar needs a label and units, i.e. log_10(objects/X) where you decide on X (square degree, square arcsec, sterad?), and compute the correct number given that NSIDE is 32 (which corresponds to a healpixel size of 1.0226539 * 10-3 sterad)
In one cell, remove the commented-out line #rand_cut = rand
Similar to Alice's comment about explaining the meaning of all symbols used in the Landy-Szalay estimator, the key function tpcf_ls()
should have a longer docstring, explaining all parameters.
Decide on one way to write this, either 'LCDM' or '\Lambda CDM' throughout the NB.
The function calc_Dv()
is defined, but not used. A bit of a letdown ;-) Would you consider either demonstrating the usage of it, or to remove it from the NB?
Hi @kadrlica @jacquesalice @rnikutta ,
Thank you all for the feedback and suggestions! I've made changes to the notebook, incorporating all the changes and fixing all the issues that were pointed out. Two notes from me:
That's it from me! Please do let me know if there is anything else I can do. Have a good weekend!
Hi @JasonWooo and @kadrlica, as a new member, I just reviewed your NB. The contents are great. Thanks for your contribution!
I have only minor suggestions:
Hi @galaxyumi,
Thank you for all the valuable feedback! I incorporated them into the notebook and pushed to my fork of notebooks-latest. For suggestion #4 in particular, I also changed all arxiv links to published versions from their respective journals.
Thank you all again for following through with this project. Please let me know if there is anything else I can do @kadrlica @rnikutta @jacquesalice
Thanks @JasonWooo ! It looks great. Merging to master now :)
Added notebook "BaryonicAcousticOscillation_BOSSDR12_20230518", which is a detailed instruction on characterizing the baryonic acoustic oscillations using SDSS DR12 data using two-point correlation functions.