To the regular wire grid calibration, the basic functions are modified/adjusted to the real site data. #1022
Conversation
hnakata-JP
requested review from
sadachi5,
ykyohei,
msilvafe,
kmharrington and
mhasself
|
Okay, I have to modify the readthedoc a little bit more, but here we should have a kind of stable module for the regular calibration. Please check it. After merging this, the development team will move to release metadata and add some scripts for its automation. |
There was a problem hiding this comment.
I petered out at the end of the review because its a lot of lines and I'm not sure what is still a work in progress. Is it still a work in progress? If not and you think this is ready to merge would you be able to present on an upcoming call your analyses using this code (i.e. how its been tested)? Thanks.
| def interpolate_hk(hk_data): | ||
| """ | ||
| Simple function to get an interpolation function with a given house-keeping data loaded by so3g.hk.load_range. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| hk_data : data | ||
| house-keeping data as the output of so3g.hk.load_range | ||
|
|
||
| Returns | ||
| ------- | ||
| interp_func : function | ||
| a function to interpolate the house-keeping data | ||
|
|
||
| """ | ||
| interp_func = {} | ||
| for key, val in hk_data.items(): | ||
| interp_func[key] = interp1d(hk_data[key][0], hk_data[key][1], bounds_error=False) | ||
| return interp_func | ||
|
|
||
|
|
||
| # interp_func is the dictionary of functions to interpolate house-keeping data | ||
| def cosamnple_hk(tod, interp_func, is_merge=False): | ||
| """ | ||
| Simple function to co-sampling house-keeping data along the timestamps in tod. This function can be used for the general usage, but currently is specified to the wire grid calibration. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| tod : AxisManager | ||
| interp_func : function | ||
| output of the interpolate_hk | ||
| is_marge : bool (default, False) | ||
| if merge the result into tod or not | ||
|
|
||
| Returns | ||
| ------- | ||
| hk_aman : AxisManager | ||
|
|
||
| """ | ||
| hk_aman = core.AxisManager(tod.samps) | ||
| for key in interp_func.keys(): | ||
| _cosamp_data = interp_func[key](tod.timestamps) | ||
| hk_aman.wrap(key, _cosamp_data, [(0, 'samps')]) | ||
| if is_merge: | ||
| tod.wrap('hk_data', hk_aman) | ||
| return hk_aman |
There was a problem hiding this comment.
This feels pretty general and not specific to the wiregrid analysis. There's this make_cosamp_hk site_pipeline script, which calls an hk_utils function get_detcosamp_hkaman is it possible to use one of these general tools?
There was a problem hiding this comment.
Maybe it's fine, but the real data will be extrapolated by the standard cosampling methods. We don't want to extend the limit switch data because it can change the ON/OFF status. I added some comments just in case. In future pull request, they can be replaced by the common one though.
| # Define house-keepind data configurations | ||
| _tel = tod.obs_info.telescope | ||
| hk_dir = '/so/level2-daq/' + _tel + '/hk' | ||
| wg_encoder_fields = { | ||
| 'enc_rad_raw': _tel + '.wg-encoder.feeds.wgencoder_full.reference_count', | ||
| } | ||
| wg_actuator_fields = { | ||
| 'LSL1': _tel + '.wg-actuator.feeds.wgactuator.limitswitch_LSL1', | ||
| 'LSL2': _tel + '.wg-actuator.feeds.wgactuator.limitswitch_LSL2', | ||
| 'LSR1': _tel + '.wg-actuator.feeds.wgactuator.limitswitch_LSR1', | ||
| 'LSR2': _tel + '.wg-actuator.feeds.wgactuator.limitswitch_LSR2', | ||
| } | ||
|
|
||
| try: | ||
| # for encoder data | ||
| _data_enc = load_range( |
There was a problem hiding this comment.
I recommend moving from load_range to using the hkdb everywhere you're using that
from sotodlib.io import hkdb
cfg = hkdb.HkConfig.from_yaml('<path_to_cfg_file>/site_hkdb_cfg.yaml')
lspec = hkdb.LoadSpec(
cfg=cfg, start=tod_start, end=tod_stop,
fields=list(wg_actuator_fields.keys()),
)
_data_enc = hkdb.load_hk(lspec, show_pb=True)
For aliases you define them in the config file see the doctstring for LoadSpec. @jlashner put in a lot of work to speed that up over load_range.
There was a problem hiding this comment.
Temporarily, I'll prepare the two kinds of wrappers. I'll leave the hkdb one as just a place holder for a while because the method is not available in NERSC yet. Somebody or probably I will replace it in the next pull request.
|
|
||
|
|
||
| # interp_func is the dictionary of functions to interpolate house-keeping data | ||
| def cosamnple_hk(tod, interp_func, is_merge=False): |
There was a problem hiding this comment.
typo cosamnple -> cosample
| wg_offset = wg_offset_satp1 | ||
| elif telescope == 'satp2': | ||
| elif _tel == 'satp2': | ||
| wg_offset = wg_offset_satp2 | ||
| elif telescope == 'satp3': | ||
| elif _tel == 'satp3': | ||
| wg_offset = wg_offset_satp3 |
There was a problem hiding this comment.
Make these capitalized i.e. wg_offset_satp1 -> WG_OFFSET_SATP1 when defined at top of the module that's been the standard for global variables for the module
| ) | ||
|
|
||
| else: | ||
| logger.warning("Define the oparation range by the encoder data instead of limit switches. \ |
There was a problem hiding this comment.
operation - mispelled in a few places.
|
|
||
|
|
||
| ## function for time constant measurement | ||
| def devide_tod(tod, forward_margin=None, backward_margin=None): |
| _flip_dir2 = np.where(tod.hwp_solution.quad_2[1:] - tod.hwp_solution.quad_2[:-1])[0] | ||
| try: | ||
| assert _flip_dir1 == _flip_dir1 | ||
| # logger here |
There was a problem hiding this comment.
add sp_util logger and remove comment.
|
|
||
| def binning_data(ref_data, target_data, num_bin=100): | ||
| """ | ||
| a function binning given data against a reference data |
There was a problem hiding this comment.
This could use a bit more explanation since its a bit different than what's in sotodlib.tod_ops.binning this is mostly for binning hk_feeds onto some timestamps it seems.
There was a problem hiding this comment.
yeah, I should add more description about it. Thanks.
|
|
||
| def _linear_model(params, xval, yval, yerr): | ||
| a, b = params[0], params[1] | ||
| model = 2*a*2*np.pi*xval + b |
There was a problem hiding this comment.
We want to keep this as it is because it is more like the mathematical representations.
| return np.array(fres), np.array(ferr), np.array(fchi2) | ||
|
|
||
|
|
||
| def get_time_constant(tod1, tod2, hwp_sign=-1, slice0=(20,-20), slice1=(10,-25), is_wrap=True): |
There was a problem hiding this comment.
nearly every line in this fn is duplicated for the 1 copy and 2 copy, I think you want a fn that does all these steps for a single tod and then this function just calls that fn twice one for each tod and wraps/returns what you need.
There was a problem hiding this comment.
I'll change it simpler. Thanks also.
|
Hi Max, thanks for a lot of useful comments! This should still have a couple of typos. but basically, the implementation is done for now. I would be happy with more reviews. |
|
I should modify the explanations in the doc page, but please ignore it for now. |
In this pull request, I adjusted the basic functions of the wire grid to the real site data. Anyone should easily use this wiregrid module with fewer bugs. The main points are listed as follows:
Please review/comment on this request. Thank you first.