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

Daily amsr2 prototype fields #140

Merged
merged 44 commits into from
Sep 17, 2024

Conversation

trey-stafford
Copy link
Contributor

@trey-stafford trey-stafford commented Sep 13, 2024

Initial code to support creating daily files with an amsr2_prototype subgroup for AMSR2 fields.

There are still a fair number of TODOs here, but they can wait for if we have more time after adding support for daily aggregate files, monthly files, validation, and NRT.

For usage, see make_25km_ecdr.sh.

Creating the AMSR2 files relies on setting an environment variable at import-time, which means that the code cannot run as a single process. First, daily data is produced using the default platform ranges. Then, a new process is spawned (see cli/daily.py) with the PLATFORM_START_DATES_CONFIG_FILEPATH set to the prototype platform start dates config (containing just AMSR2), which produces the amsr2 fields. Finally, the files get staged for publication by combining the F17 outputs w/ AMSR2 outputs in the prototype_amsr2 group (publish_daily.py).

Longer term, a complete refactor of the code to support this scheme from foundations would be ideal. We could have a single config file with cdr_platform_start_dates and prototype_platform_start_dates keys that the program would know how to parse and act accordingly without needing to spawn separate processes to access different platform configs. We don't have the time for that now, and the present approach is a byproduct of development that originally did not have this in mind (we were going to produce separate 12.5km data files for AMSR2 instead of adding it as a "prototype" group to F17 output files - just a single platform start date config).

@trey-stafford trey-stafford changed the base branch from main to revert-143-revert-142-update-pm_icecon September 13, 2024 22:53
@trey-stafford trey-stafford force-pushed the combine-amsr2-prototype-fields branch from 7b9ab90 to 858e7ec Compare September 13, 2024 23:24
@trey-stafford trey-stafford changed the base branch from revert-143-revert-142-update-pm_icecon to ecdr-at-25km September 16, 2024 15:01
@trey-stafford trey-stafford force-pushed the combine-amsr2-prototype-fields branch from af998d4 to 6cf10f2 Compare September 17, 2024 02:04
@trey-stafford trey-stafford marked this pull request as ready for review September 17, 2024 15:22
hemisphere=hemisphere,
is_nrt=False,
)
cdr_var_fieldnames = [
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh man, this list looks so NICE compared to prior versions!

config = PlatformConfig(
platforms=[
Platform(
id="ame",
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we're allowing AMSR-E, are we?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not yet (maybe not ever?) but the code exists to support it. This unit test just uses ame as an example.

Copy link
Contributor

@sc0tts sc0tts left a comment

Choose a reason for hiding this comment

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

I added some single-comments to specific locations in the code where I think we should add "TODO" statements to remind us to replace hard-coded values with configuration values.

Also, there is mention of AMSR-E at at least one place in the code. Is that OBE now?

But these are minor and should not hold up the merging of this valuable PR!

@trey-stafford trey-stafford merged commit d3c7919 into ecdr-at-25km Sep 17, 2024
1 check passed
@trey-stafford trey-stafford deleted the combine-amsr2-prototype-fields branch September 17, 2024 17:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants