Lo L1C - Automatic Goodtimes#2896
Lo L1C - Automatic Goodtimes#2896sdhoyt wants to merge 4 commits intoIMAP-Science-Operations-Center:devfrom
Conversation
…C compatible CDF production code.
ahotasu
left a comment
There was a problem hiding this comment.
Looks good to me--all of my comments are for long-term polish and aren't needed prior to merge.
| epochs[row_count] = l1b_histrates["epoch"][max_row_count].values.item() | ||
| goodtimes[row_count, :] = [begin, end] | ||
| epochs[row_count] = l1b_histrates["epoch"][max_row_count - 1] | ||
| goodtimes[row_count, :] = [int(begin - 620), int(end + 320)] |
subagonsouth
left a comment
There was a problem hiding this comment.
A few suggestions for improvements. Looks good other than these minor things.
| if hasattr(shcoarse, "values"): | ||
| shcoarse = shcoarse.values | ||
| shcoarse = np.asarray(shcoarse, dtype=np.float64) | ||
| # shcoarse = epochs_ttj2000 / 1e9 # Convert from ns to s MET |
There was a problem hiding this comment.
| # shcoarse = epochs_ttj2000 / 1e9 # Convert from ns to s MET |
| # We use current SPICE kernels, so we should NOT add that offset | ||
| shcoarse = ttj2000ns_to_met(epochs_ttj2000) | ||
| # Convert to plain numpy array for easier indexing | ||
| if hasattr(shcoarse, "values"): |
There was a problem hiding this comment.
Interesting. Is this because ttj2000ns_to_met was returning an xr.DataArray? I didn't know it would do that. You could just pass in l1b_histrates["epoch"].values instead.
|
|
||
| h_intensity = np.sum(l1b_histrates["h_counts"][:, 0:7, 20:50], axis=(1, 2)) | ||
| o_intensity = np.sum(l1b_histrates["o_counts"][:, 0:7, 20:50], axis=(1, 2)) | ||
| epochs_ttj2000 = l1b_histrates["epoch"][:] |
There was a problem hiding this comment.
What is the [:] doing here?
| epochs = l1b_histrates["epoch"].values | ||
| epochs = xr.DataArray(epochs, dims=["epoch"]) | ||
| goodtimes = xr.DataArray(np.zeros((max_row_count, 2), dtype=np.int64)) | ||
| h_background_rate = xr.DataArray(np.zeros((max_row_count, 7), dtype=np.float32)) |
There was a problem hiding this comment.
Lots of hard coded sevens in this section of code. Do you have a global variable somewhere that defines the number of ESA steps that could be used instead? Otherwise, maybe use a module scoped global?
| # Note: The reference script adds +9 seconds because they use an | ||
| # "older time kernel (pre 2012)" | ||
| # We use current SPICE kernels, so we should NOT add that offset | ||
| shcoarse = ttj2000ns_to_met(epochs_ttj2000) |
There was a problem hiding this comment.
shcoarse is typically reserved for integer spacecraft clock coarse ticks. I suggest renaming to "met" or similar.
| h_background_rate[row_count, :] = [ | ||
| h_bg_rate, | ||
| h_bg_rate, | ||
| h_bg_rate, | ||
| h_bg_rate, | ||
| h_bg_rate, | ||
| h_bg_rate, | ||
| h_bg_rate, | ||
| ] |
There was a problem hiding this comment.
I think all these repititions of values could be done like:
| h_background_rate[row_count, :] = [ | |
| h_bg_rate, | |
| h_bg_rate, | |
| h_bg_rate, | |
| h_bg_rate, | |
| h_bg_rate, | |
| h_bg_rate, | |
| h_bg_rate, | |
| ] | |
| h_background_rate[row_count, :] = np.full(num_esa_steps, h_bg_rate) |
Change Summary
Overview
This code integrates Lo's python code for automatic goodtimes into the processing code.
I'm running into a mypy issue because of the number of if statements in a function. I'd like to push fixing that to another ticket a little later down the road to prioritize the remaining work I have for the 3 month maps.
I compared goodtimes values to the results of Lo's script. There was divergence, up to a few hundred seconds. If this needs to be more accurate, I'd also propose pushing this to a later change.
Closes #2841