Codecompare #29
Codecompare #29
Conversation
…ug corrected in ccl_power concerning the normalization of the BBKS power spectrum. Makefile now includes BBKS test, but this test doesnt yet pass.
|
This is where you put general comments. |
| i++; | ||
| } | ||
| } | ||
| /* |
There was a problem hiding this comment.
Should remove these commented out bits if they are not useful.
| @@ -297,8 +297,8 @@ void ccl_cosmology_compute_power_bbks(ccl_cosmology * cosmo, int *status){ | |||
| } | |||
| gsl_spline * log_power_lin = gsl_spline_alloc(K_SPLINE_TYPE, nk); | |||
| *status = gsl_spline_init(log_power_lin, x, y, nk); | |||
| double sigma_8 = ccl_sigma8(log_power_lin, cosmo->params.h, status); | |||
| double log_sigma_8 = log(cosmo->params.sigma_8) - log(sigma_8); | |||
| double sigma_8 = ccl_sigma8(log_power_lin, cosmo->params.h, status); //this is sigma_8^2! | |||
There was a problem hiding this comment.
Should we change ccl_sigma8 to return sigma_8, not sigma_8^2 to avoid confusion?
| //TODO: Also, what units is k? If [k]=Mpc/h, then we should remove h from kR. | ||
| //TODO: It seems in the constants.h file thtat [k]=Mpc | ||
| //TODO: It seems in the constants.h file that [k]=Mpc |
There was a problem hiding this comment.
Need to revisit this when checking physical units
| double mu = 5*log10(DL_pc)-5; | ||
| //double DL = 1/h * ccl_luminosity_distance(cosmo, a); | ||
| //double DL_pc = DL*1e6; | ||
| //double mu = 5*log10(DL_pc)-5; |
There was a problem hiding this comment.
Same as Joe said above - removed these if no longer useful?
| double mu = 5*log10(DL_pc)-5; | ||
| //double DL = 1/h * ccl_luminosity_distance(cosmo, a); | ||
| //double DL_pc = DL*1e6; | ||
| //double mu = 5*log10(DL_pc)-5; |
There was a problem hiding this comment.
Same as Joe said above - removed these if no longer useful?
|
As a matter of practice, I recommend not merging pull requests that have open questions/suggestions on the code in future. It's easier to iterate on any changes while the code remains on an unmerged branch. |
|
To help a bit with this, I just went to the settings for CCL and turned on protection for the master branch: now, pull requests must be reviewed and approved before they can be merged. The admins of this repo (currently @elikrause @joachimi @damonge @elisachisari ) can change this setting as you like. (You can also add other things, like requiring status checks to pass etc. This is a good idea for when your unit tests are up and running). BTW, I notice in the "Collaborators and Teams" section of this repo's settings that you are using the "collaborators" facility to enable write access for a large group of people. I suggest that one of you admins instead makes a team, called "CCL-developers" or something, and add these people to it before removing them as "collaborators". The reason is that you can then talk to each other better: if someone needs to say something that all developers need to hear, you can @mention the team name and they will get a "mention" notification (as opposed to a regular "watching" notification). The "collaborators" type is best reserved for people who are not members of the organization, I think - it certainly makes it easy to see who those people are! |
This branch incorporates the outputs of the code comparison project and runs tests comparing the output of the CCL routines to those outputs.