Skip to content
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

Initial CoDICE L1A Processing Algorithm #288

Merged
merged 17 commits into from
Nov 30, 2023

Conversation

bourque
Copy link
Collaborator

@bourque bourque commented Nov 28, 2023

Change Summary

Overview

This PR includes an initial pass at CoDICE L1A processing. Currently it only supports/works on CoDICE housekeeping packets, and does not yet do any analog to EU conversions (that work will be in a future PR and is currently being done by @GFMoraga in #274). The code is heavily inspired by @tech3371's SWE processing code.

The PR also includes a re-organization of the CoDICE code as a whole. Basically I moved all processing code to be directly under the codice/ directory instead of having separate l0/ and l1a/ directories. This organization makes most sense to me in my head, but I'm definitely open to other ideas. I also tried renaming some files so that they are a bit less repetitive.

Lastly, I added/updated some module and function docstrings, and updated the CoDICE API reference docs.

New Dependencies

None

New Files

  • /cdf/utils.py
    • Added condition to set the file_start_date for CoDICE files
  • /codice/__init__.py
  • /codice/codice_cdf_attrs.py
    • CDF attributes for CoDICE; basically just used what SWE had and replaced where appropriate
  • /codice/codice_l1.py
    • L1A processing code
  • /codice/tests/test_codice_l1a.py
    • Tests for L1A processing code

Deleted Files

None

Updated Files

  • /docs/source/reference/codice.rst
    • Added new L1A reference documentation and made changes to reflect the reorganization of files
  • /codice/codice_l0.py
    • Moved from imap_processing/codice/l0/decom_codice.py
  • /codice/utils/decompress.py
    • Moved from imap_processing/codice/l0/decompress_codice.py
  • /codice/tests/test_codice_l0.py
    • Minor formatting updates
  • /codice/utils/codice_utils.py
    • Added some functions to support L1A processing

Testing

I added unit tests for L1A processing, but currently only tests on housekeeping packets.

@bourque bourque added Ins: CoDICE Related to the CoDICE instrument Level: L1 Level 1 processing labels Nov 28, 2023
@bourque bourque self-assigned this Nov 28, 2023
@bourque bourque requested review from a team, sdhoyt, greglucas, tech3371, bryan-harter, laspsandoval, GFMoraga and maxinelasp and removed request for a team November 28, 2023 23:20
Copy link
Contributor

@maxinelasp maxinelasp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, this looks great

Copy link
Contributor

@GFMoraga GFMoraga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great! I will have to re-base but I think all of your updates and naming conventions are great!

imap_processing/codice/codice_cdf_attrs.py Outdated Show resolved Hide resolved
imap_processing/codice/utils/codice_utils.py Outdated Show resolved Hide resolved
@bourque
Copy link
Collaborator Author

bourque commented Nov 29, 2023

@GFMoraga Yeah, this will likely cause you some merge conflicts, but I can help resolve those since they will be my fault!

Copy link
Collaborator

@greglucas greglucas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, looks great! Just minor comments from me, but would prefer the test cleanup one to be added if possible.

I also personally like this structure better, I find the multiple nested modules a little harder to follow. If you're working on naming I might even suggest dropping the codice from the filenames within your codice module since that is repetitive. Something like this with a flatter instrument structure. (But, that is just my personal preference, no need to take it if you disagree)

codice/
    cdf_attrs.py
    level0.py
    level1a.py
    decommutation.py
    utils.py

imap_processing/cdf/utils.py Outdated Show resolved Hide resolved
imap_processing/codice/codice_cdf_attrs.py Outdated Show resolved Hide resolved
imap_processing/codice/codice_cdf_attrs.py Outdated Show resolved Hide resolved
imap_processing/codice/codice_cdf_attrs.py Outdated Show resolved Hide resolved
imap_processing/codice/codice_cdf_attrs.py Outdated Show resolved Hide resolved
Comment on lines 60 to 62
validmin=0,
validmax=GlobalConstants.INT_MAXVAL,
format="I12",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity, do we care that the maxval is a 64-bit signed integer, but we have only 12 digits of representation in the format?

INT_MAXVAL = np.iinfo(np.int64).max

imap_processing/codice/codice_l1a.py Show resolved Hide resolved
imap_processing/codice/tests/test_codice_l1a.py Outdated Show resolved Hide resolved
imap_processing/codice/utils/codice_utils.py Outdated Show resolved Hide resolved
@bourque
Copy link
Collaborator Author

bourque commented Nov 29, 2023

I can't for the life of me figure out why the doc build is failing. There are tons of WARNINGs (which is its own problem) but I don't think those should cause the entire build to fail. I don't see any actual ERRORs.

@maxinelasp
Copy link
Contributor

I can't for the life of me figure out why the doc build is failing. There are tons of WARNINGs (which is its own problem) but I don't think those should cause the entire build to fail. I don't see any actual ERRORs.

I think warnings do cause the check to fail. You can try updating the config.py file like what I have in my currently open glows PR, and I think you will also need a Methods block in your class docs. I have a docfix commit on glows l1a which fixes very similar errors from my doc warning, so it might help you to look there.

@bourque
Copy link
Collaborator Author

bourque commented Nov 30, 2023

Thanks @maxinelasp!

@bourque bourque requested review from greglucas and a team November 30, 2023 17:06
Copy link
Contributor

@tech3371 tech3371 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work!

imap_processing/cdf/utils.py Outdated Show resolved Hide resolved
imap_processing/codice/utils.py Outdated Show resolved Hide resolved
imap_processing/codice/codice_l1a.py Show resolved Hide resolved
@bourque bourque merged commit 59b42d3 into IMAP-Science-Operations-Center:dev Nov 30, 2023
14 checks passed
@bourque bourque deleted the initial-codice-l1a branch November 30, 2023 21:54
laspsandoval pushed a commit to laspsandoval/imap_processing that referenced this pull request Apr 2, 2024
…itial-codice-l1a

Initial CoDICE L1A Processing Algorithm
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ins: CoDICE Related to the CoDICE instrument Level: L1 Level 1 processing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants