RX Channel Imbalances in EAP #192
Conversation
rad-eng-59
requested review from
Tyler-g-hudson,
bhawkins,
hb6688 and
hfattahi
Tyler-g-hudson
left a comment
Tyler-g-hudson
left a comment
There was a problem hiding this comment.
Some code organization and documentation commentary
| # XXX perhaps Caltone frequency can be parsed from L0B DRT | ||
| # rather than provided as an input! |
There was a problem hiding this comment.
Is resolving this within the scope of this PR, or would we need to wait until later?
| self.finder[rxpol] = TimingFinder(self.pulse_times, rd_all[rxpol], | ||
| wd_all[rxpol], wl_all[rxpol]) | ||
| wd_all[rxpol], wl_all[rxpol]) | ||
| # XXX get range delay offset for band B in split-spectrum to |
There was a problem hiding this comment.
Why is this explanatory comment marked with XXX?
|
|
||
|
|
||
| @dataclass(frozen=True) | ||
| class RX_CHANNEL_IMBALANCE_PRODUCT: |
There was a problem hiding this comment.
| class RX_CHANNEL_IMBALANCE_PRODUCT: | |
| class RxChannelImbalanceProduct: |
All-caps names like these are typically reserved for constants in Python, is there a reason why you named the class in this way?
| time_delays_sec: np.ndarray | ||
| max_amp_ratio: float | ||
|
|
||
| def __post_init__(self): |
There was a problem hiding this comment.
Maybe check the array dtypes here to ensure they're complex/int/float respectively?
| # XXX Size of all arrays must be 12 for L-band NISAR but | ||
| # not enforced due to failure of special cases such as unit test |
There was a problem hiding this comment.
Maybe warn if this isn't the case?
| caltone_freq = parse_caltone_freq_from_drt(raw, txrx_pol) | ||
| warn(f'Caltone frequency is extracted from {txrx_pol[1]}-pol DRT ' | ||
| f'-> {caltone_freq * 1e-6:.3f} (MHz)') | ||
| # XXX check if product from the second band so we can modify |
There was a problem hiding this comment.
This appears to be a descriptive comment, why is it marked with # XXX?
| lna_caltone_ratio, dif_chirp_caltone_freq) | ||
| if _is_product_from_second_band(raw, freq_band, txrx_pol): | ||
| warn(f'correcting LNA/CALTONE for band={freq_band} and pol={txrx_pol}') | ||
| # if there is then get diff of frequency bands A dn B |
There was a problem hiding this comment.
| # if there is then get diff of frequency bands A dn B | |
| # if there is then get diff of frequency bands A and B |
| if freq_band == "B" and len(raw.frequencies) == 2: | ||
| if txrx_pol in raw.polarizations['A']: | ||
| return True | ||
| return False | ||
| return False |
There was a problem hiding this comment.
| if freq_band == "B" and len(raw.frequencies) == 2: | |
| if txrx_pol in raw.polarizations['A']: | |
| return True | |
| return False | |
| return False | |
| if freq_band == "B" and len(raw.frequencies) == 2: | |
| if txrx_pol in raw.polarizations['A']: | |
| return True | |
| return False |
Nitpick: Only one return False is necessary since the code will exit the conditional block anyway
| get time delays for a qfSP with phase anomaly only for | ||
| 12 channel NISAR L-band product. | ||
| For other case, it will be set to zero! |
There was a problem hiding this comment.
| get time delays for a qfSP with phase anomaly only for | |
| 12 channel NISAR L-band product. | |
| For other case, it will be set to zero! | |
| If the product is a 12-channel NISAR L-band product, | |
| return the time delays for a qFSP with phase anomaly. | |
| Else, return zeros. |
| # XXX to avoid unit test failure and old sim L0B | ||
| # a warning will be issued and all values will be set | ||
| # to unity! |
There was a problem hiding this comment.
Similar concern to other XXX comments - is this behavior undesirable? Do we intend to change it later?
2 participants
This PR computes frequency-dependent polarization-dependent RX channels imbalances and incorporate them into RX DBF process of pattern.py module as part EAP in RSLC workflow.
By adding this feature:
Disclaimer: Not yet fully tested over a pair of L0Bs with phase jump issue!