Skip to content

compile script reorganization#114

Merged
btobers merged 9 commits intodevfrom
113_compile_reorg
Jun 10, 2025
Merged

compile script reorganization#114
btobers merged 9 commits intodevfrom
113_compile_reorg

Conversation

@btobers
Copy link
Collaborator

@btobers btobers commented Jun 3, 2025

Closes #113.

Dictionary structures used to initialize and aggregate regional results.

@btobers btobers self-assigned this Jun 3, 2025
@btobers btobers marked this pull request as ready for review June 4, 2025 15:34
@btobers btobers requested a review from a team June 4, 2025 15:34
Copy link
Member

@yelizy yelizy left a comment

Choose a reason for hiding this comment

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

Neat! I haven't tried running the code but it looked okay to me. I am wondering if there is a way to reduce the use of elif statements for each variables to assign their attributes.

@btobers
Copy link
Collaborator Author

btobers commented Jun 6, 2025

Neat! I haven't tried running the code but it looked okay to me. I am wondering if there is a way to reduce the use of elif statements for each variables to assign their attributes.

Yes, maybe a dictionary structure with the per-variable metadata that can be indexed when building the datasets. That'd certainly help clean up this script. I'm sure it can be further improved in the future.

@btobers
Copy link
Collaborator Author

btobers commented Jun 6, 2025

As of now, there's no way to autmoatically test the postproc_compile_simulations script, since the test notebooks which are run automatically only run single glaciers and don't compile any regional results. Per #50, we should add a test for this script. In the meantime, I have run compilation locally to verify this is working as expected.

@btobers
Copy link
Collaborator Author

btobers commented Jun 6, 2025

Decided to create a compile test script (tests/test_04_postproc.py) which is now included in the Git CLI test workflow. This relatively simple test script post-processes the monthly mass (calling postproc/postproc_monthly_mass.py and then compiles region 15 results (calling postproc/postproc_compile_simulations.py) using the results of PyGEM-notebooks/simple_test.ipynb for RGI60-15.03733. So we now hit two more postprocessing scripts in our CLI workflow testing. I think this can now be approved by @PyGEM-Community/dev and we can continue to build on it and improve in the future.

@btobers btobers requested a review from drounce June 10, 2025 18:04
@btobers btobers merged commit 43ef38a into dev Jun 10, 2025
1 check passed
@btobers btobers deleted the 113_compile_reorg branch June 10, 2025 19:35
btobers added a commit that referenced this pull request Aug 19, 2025
Closes #113.
This PR modifies the postprocessing simulation compilation script to use dictionary structures in compiling variables by region, GCM, scenario, etc, thereby simplifying the script quite a bit.

This PR also partially addresses #50 by adding postprocessing scripts to the CLI testing workflow.
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.

3 participants