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

Bug fix for jobs with multiple grids in the same file. #117

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ledm
Copy link
Collaborator

@ledm ledm commented Nov 14, 2023

So, there are a bunch of NEMO jobs with data in the wrong file.

 ncdump -h /gws/nopw/j04/ukesm/BGC_data/u-ct598/nemo_ct598o_1y_19811201-19821201_grid-T.nc | grep 'area'
        float area_grid_T(y_grid_T, x_grid_T) ;
                area_grid_T:standard_name = "cell_area" ;
                area_grid_T:units = "m2" ;
        float area_grid_W(y_grid_W, x_grid_W) ;
                area_grid_W:standard_name = "cell_area" ;
                area_grid_W:units = "m2" ;

Basically, someone added data from two different grids to this file, so it has to include both grid definitions. It also has two sets of nav_lat and nav_lon. Instead of failing at run time, and outputting an error message, crazily NEMO allows this and it messes with everything at the output stage. It's a broken NEMO system and it was set up incorrectly.

It outputs the error:

  File "/home/users/ayool/miniconda3/envs/bgcval2/bin/analysis_compare", line 8, in <module>
    sys.exit(main())
             ^^^^^^
  File "/home/users/ayool/bgcval2/bgcval2/analysis_compare.py", line 688, in main
    load_yml_and_run(compare_yml, config_user, skip_timeseries)
  File "/home/users/ayool/bgcval2/bgcval2/analysis_compare.py", line 603, in load_yml_and_run
    analysis_timeseries(
  File "/home/users/ayool/bgcval2/bgcval2/analysis_timeseries.py", line 687, in analysis_timeseries
    tsa = timeseriesAnalysis(
          ^^^^^^^^^^^^^^^^^^^
  File "/home/users/ayool/bgcval2/bgcval2/timeseries/timeseriesAnalysis.py", line 121, in __init__
    self.loadModel()
  File "/home/users/ayool/bgcval2/bgcval2/timeseries/timeseriesAnalysis.py", line 262, in loadModel
    DL = tst.DataLoader(
         ^^^^^^^^^^^^^^^
  File "/home/users/ayool/bgcval2/bgcval2/timeseries/timeseriesTools.py", line 280, in __init__
    if data == '': data = std_extractData(nc, self.details)
                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/users/ayool/bgcval2/bgcval2/functions/standard_functions.py", line 62, in extractData
    xd = details['convert'](nc,details['vars'], **kwargs)
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/users/ayool/bgcval2/bgcval2/functions/globalVolMean.py", line 62, in globalVolumeMean
    volume = calc_vol(nc)
             ^^^^^^^^^^^^
  File "/home/users/ayool/bgcval2/bgcval2/functions/globalVolMean.py", line 40, in calc_vol
    area = nc.variables['area'][:]
           ~~~~~~~~~~~~^^^^^^^^
KeyError: 'area'

This patch solves this problem.

@ledm ledm requested a review from DrYool November 14, 2023 16:57
@ledm
Copy link
Collaborator Author

ledm commented Nov 14, 2023

@DrYool please let me know when you're tested this.

@ledm ledm requested review from jupalm and valeriupredoi November 14, 2023 16:58
Copy link
Owner

@valeriupredoi valeriupredoi left a comment

Choose a reason for hiding this comment

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

LGTM 🍺

bgcval2/functions/globalVolMean.py Outdated Show resolved Hide resolved
@valeriupredoi
Copy link
Owner

your sacrificial commit made me realize you can tag GH users in commit messages and they get email alerts bc of that - hmm what a great idea to pester annoying reviewers 😁

@ledm
Copy link
Collaborator Author

ledm commented Nov 15, 2023

your sacrificial commit made me realize you can tag GH users in commit messages and they get email alerts bc of that - hmm what a great idea to pester annoying reviewers 😁

Your words, lol.

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