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

Analysis updates #11

Merged
merged 72 commits into from
Feb 3, 2025
Merged

Conversation

ceblanton
Copy link
Contributor

@ceblanton ceblanton commented Aug 19, 2024

  1. Add build and publish graph for analysis scripts that specify them; done
  2. Generate catalog before running analysis; done
  3. Remove mdtf as a standalone task; done
  4. Add build and publish task definitions; not quite working
  5. Use analysis yaml instead of the analysis/rose-app.conf

Chris Blanton and others added 20 commits July 12, 2024 17:13
But only for analysis scripts that use timeseries.
…ions

For now, keep the task definition in flow.cylc but it can be elsewhere.
@ceblanton
Copy link
Contributor Author

tmprcwu45qs

do analysis only option turned on

@ceblanton
Copy link
Contributor Author

tmpxqnz0v9t

do analysis only option turned off. normal graph, a bit overwhelming

3 years for both

Chris Blanton and others added 21 commits January 7, 2025 20:03
…-script.

Remaining rose apps should set the definition themselves.
- move module loads to ppan site file
- install analysis virtual envs to a subdirectory
- add one more fre analysis env var expected by scripts
- install analysis envs in a subdirectory
- parse legacy analysis script options
Non-accumulative timeaverage requests must conform to one of the chunks
in order to pass a single time to the script in "in_data_file"
…d Bronx frequency mappings for AM5 scripts
Set them to yr1 and yr2, though they have different meaning in Bronx
in order to choose the most appropriate one for each script.
Copy link
Member

@ilaflott ilaflott left a comment

Choose a reason for hiding this comment

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

not going to approve/disapprove quite yet cause this is still a draft- but wanted to provide some prelim feedback!

@@ -1,5 +1,8 @@
#!jinja2

{# fre version should be programatically set, not hard-coded. #}
{% set FRE_VERSION = 2025.01 %}
Copy link
Member

Choose a reason for hiding this comment

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

this is a "fine-for-now". In the future, a Jinja2Filter should assign this via import fre; fre.version;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great point, thank you. You make it sound quite doable! We should do it right away if it's only three words like that.

@ceblanton ceblanton marked this pull request as ready for review February 3, 2025 19:20
Copy link
Member

@ilaflott ilaflott left a comment

Choose a reason for hiding this comment

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

LGTM/Approving! We need real-use feedback, any nits i pointed out are fine for now. the switch DO_ANALYSIS exists for a reason and we can always shut things down if they are problematic.

@ilaflott ilaflott merged commit 0a00339 into NOAA-GFDL:main Feb 3, 2025
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