-
Notifications
You must be signed in to change notification settings - Fork 22
CoDICE: Production preparation #2562
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
CoDICE: Production preparation #2562
Conversation
| <!-- TODO: undo this in coming DE segmented work --> | ||
| <!-- <xtce:IntegerParameterType name="COD_LO_PHA.SHCOARSE" signed="false"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are you undoing this change? Can't we leave it in if you are skipping DEs.
I think we should have the XTCE be what it is and not comment things out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so keep it as we want in the upcoming work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see what you mean. I will uncomment these
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I would prefer the XTCE to be exactly what I put in the other branch.
Although now I'm wondering if the tests will work with that update or not...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True..the tests will fail but I need to skip them anyway. So your suggestion works for me still.
I haven't done the thorough tests yet because I am getting this draft PR up so that Luisa can test in dev if she gets chance today. Then I will resume dev testing my tomorrow.
tech3371
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some comments for ourselves.
| # TODO: Handle epoch dependent acquisition time and half spin per esa step | ||
| # For now, just tile the same array for all epochs. | ||
| # Eventually we may have data from a day where the LUT changed. If this is the | ||
| # case, we need to split the data by epoch and assign different acquisition times | ||
| half_spin_per_esa_step = np.tile( | ||
| np.asarray(half_spin_per_esa_step), | ||
| (len(unpacked_dataset["acq_start_seconds"]), 1), | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the case for when we have two SCI-LUT. When this happens, we need to use correct SCI-LUT for given table_id.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why the name half_spin_per_esa_step with (epoch, esa_step) dims? CoDICE team had that. But this still time varying but the value of this variable is row_number from lo_stepping_tab.
how is this different from acquisition time? This is just recording the row_number. This is needed for L2.
use time to get correct SCI-LUT to get correct row_number data.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. An example date where we expect this to happen is dec 18th
|
|
||
|
|
||
| def calculate_acq_time_per_step(low_stepping_tab: dict) -> np.ndarray: | ||
| def calculate_acq_time_per_step( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will get effected from two lut in one day data.
| spin_sector_len = constants.LO_DESPIN_SPIN_SECTORS | ||
| despun_shape = (num_packets, num_species, esa_steps, spin_sector_len, inst_az_dim) | ||
| despun_data = np.full(despun_shape, 0) | ||
| despun_data = np.full(despun_shape, np.nan) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is causing unit test failures. Are you sure this is supposed to be nan?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
its also causing validation against the dec 10th data to break. I think you're right here to use np.nan so maybe we should bring this up in the tagup tomorrow.
Change Summary
Overview
This code changes gets CoDICE non-DE products ready for production. DE preparation will come later.
NOTE:
I checked the CLI and they seem ready. I feel like this is all we need at the moment but dev test with latest LUT file will tell us. I am keeping this in draft PR until dev test is ok.