Stellar mass function test #51
Conversation
|
Happy to see |
There was a problem hiding this comment.
@evevkovacs thanks. Mostly minor style stuffs (taken from here).
Can you also post a link to the DESCQA web interface of a run for this test?
descqa/StellarMassFunction.py
Outdated
|
|
||
| @staticmethod | ||
| def get_sky_volume(catalog_instance, zhi, zlo=0., distance='Mpc'): | ||
| from astropy.cosmology import FlatLambdaCDM |
There was a problem hiding this comment.
Move import statement to the beginning of the module
descqa/StellarMassFunction.py
Outdated
| 'missingdata':'...' | ||
| }, | ||
| } | ||
| zkey_match ='< z <' |
descqa/StellarMassFunction.py
Outdated
| 'PRIMUS_2013': { | ||
| 'filename_template': 'SMF/moustakas_et_al_2013/Table{}.txt', | ||
| 'file-info': {'0. < z < .1':{'zlo':0., 'zhi':.1, 'table#':3, 'usecols':[0,1,2,3]}, | ||
| '.2 < z < .3':{'zlo':.2, 'zhi':.3, 'table#':4, 'usecols':[0,1,2,3]}, |
There was a problem hiding this comment.
minor but aligning the first quote symbol in this and following lines would make this look nicer
descqa/StellarMassFunction.py
Outdated
| msize = 4 #marker-size | ||
|
|
||
| def __init__(self, z='redshift_true', mass='stellar_mass', Nbins=25, log_Mlo=8., log_Mhi=12., | ||
| observation='', zlo=0., zhi=1.0, zint =0.2, ncolumns=2, **kwargs): |
There was a problem hiding this comment.
remove the space right after zint
descqa/StellarMassFunction.py
Outdated
|
|
||
| def init_plots(self, zlo, zhi, zint): | ||
| #get magnitude cuts based on validation data or default limits (only catalog data plotted) | ||
| if len(self.validation_data )> 0: |
There was a problem hiding this comment.
change to if not self.validation_data:
There was a problem hiding this comment.
Done, but flipped the order around so it would work correctly.
descqa/StellarMassFunction.py
Outdated
| def run_on_single_catalog(self, catalog_instance, catalog_name, output_dir): | ||
| #update color and marker to preserve catalog colors and markers across tests | ||
| catalog_color = next(self._color_iterator) | ||
| catalog_marker= next(self.markers) |
descqa/StellarMassFunction.py
Outdated
|
|
||
|
|
||
| @staticmethod | ||
| def get_sky_volume(catalog_instance, zhi, zlo=0., distance='Mpc'): |
There was a problem hiding this comment.
distance is not used in this function
There was a problem hiding this comment.
OK I changed distance to units and added a check for units=='Mpc'. If this is going to be a function in a utilities module, we could have something more bulletproof than this. Lots of test developers will need a "get_sky_volume" function. It would save everyone doing it for themselves and will help with minimizing errors if we offer it, I think.
descqa/StellarMassFunction.py
Outdated
| @@ -0,0 +1,353 @@ | |||
| from __future__ import print_function, division, unicode_literals, absolute_import | |||
| import os | |||
| import math | |||
There was a problem hiding this comment.
remove this line because math is not used.
descqa/StellarMassFunction.py
Outdated
|
|
||
| from .base import BaseValidationTest, TestResult | ||
| from .plotting import plt | ||
| from itertools import cycle |
There was a problem hiding this comment.
move these two lines (from itertools import cycle and from itertools import count) to before import numpy as np
descqa/StellarMassFunction.py
Outdated
| sumM += np.histogram(M_this, bins=self.Mbins, weights=M_this)[0] | ||
| sumM2 += np.histogram(M_this, bins=self.Mbins, weights=M_this**2)[0] | ||
|
|
||
| if not N.sum(): |
There was a problem hiding this comment.
I understand what this is doing, but I wonder if we should raise an error instead of skip the catalog?
There was a problem hiding this comment.
Yes, I agree. If the catalog claims to have a quantity but nothing is there (eg dc1) then we should know. Also the test condition was wrong, so I fixed it.
|
OK, I made all the changes and added a couple of minor improvements. I also checked that the code runs with no observation Latest test run: []https://portal.nersc.gov/project/lsst/descqa/v2/?run=2017-12-21_14 and run without observation is on []https://portal.nersc.gov/project/lsst/descqa/v2/?run=2017-12-21_17 |
|
@evevkovacs thanks, remember to commit your changes to this branch! |
|
OK done. |
The last 2 functions in the test could go into a utilities module as they are more generally useful. Here's the link to the last run before the pull request.
[]https://portal.nersc.gov/project/lsst/descqa/v2/?run=2017-12-20_36&test=SMF_PRIMUS