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

Refactor run_bufr2ioda.sh as run_bufr2ioda.py #599

Closed
RussTreadon-NOAA opened this issue Sep 1, 2023 · 4 comments · Fixed by #601
Closed

Refactor run_bufr2ioda.sh as run_bufr2ioda.py #599

RussTreadon-NOAA opened this issue Sep 1, 2023 · 4 comments · Fixed by #601
Assignees

Comments

@RussTreadon-NOAA
Copy link
Contributor

g-w PR #1826 states that all new scripts added to g-w need to be in python.

GDASApp ush/ioda/bufr2ioda/run_bufr2ioda.sh is a bash script executed from g-w jobs/JGLOBAL_PREP_ATM_IODA_OBS. As such, g-w requires this script to python, not bash.

This issue is opened to document the refactoring of run_bufr2ioda.sh to run_bufr2ioda.py.

@RussTreadon-NOAA
Copy link
Contributor Author

Create feature/bufr2iodapy as copy of develop at 3146e9d.

@RussTreadon-NOAA
Copy link
Contributor Author

To accelerate refactoring, create ChatGPT account with OpenAI. Use ChatGPT to create python equivalent of run_bufr2ioda.sh Tested resulting run_bufr2ioda.py on Hera in 20210814 18 gdas cycle of pratmos.

Job prepatmiodaobs failed because environment variable DUMP was not defined.

^[[38;21m2023-09-01 14:07:36,052 - INFO     - BUFR2IODA_satwind_amv_goes.py: Making QuerySet^[[0m
^[[38;21m2023-09-01 14:07:36,054 - INFO     - BUFR2IODA_satwind_amv_goes.py: Executing QuerySet to get ResultSet^[[0m
forrtl: No such file or directory
forrtl: severe (29): file not found, unit 12, file /scratch1/NCEPDEV/global/glopara/dump/{{ DUMP }}.20210814/18/atmos/{{ DUMP }}.t18z.satwnd.\
tm00.bufr_d

GDASApp parm/ioda/bufr2ioda/bufr2ioda_satwind_amv_goes.json references variable DUMP as shown below

{
  "data_format"      : "bufr_d",
  "data_type"        : "satwnd",
  "cycle_type"       : "{{ DUMP }}",
  "cycle_datetime"   : "{{ current_cycle | to_YMDH }}",
  "dump_directory"   : "{{ DMPDIR }}",
  "ioda_directory"   : "{{ COM_OBS }}",

JGLOBAL_ATM_PREP_IODA_OBS invokes run_bufr2ioda.sh via

 # Run relevant script
 EXSCRIPT=${BUFR2IODASH:-${HOMEgfs}/ush/run_bufr2ioda.sh}
 ${EXSCRIPT} "${PDY}${cyc}" "${CDUMP}" "${DMPDIR}" "${IODAPARM}" "${COM_OBS}/"

The second argument, CDUMP is used to set variable DUMP in run_bufr2ioda.sh as shown from the snippet below

# some of these need exported to be picked up by the python script below
# input parameters
CDATE=${CDATE:-$1}
export DUMP=${RUN:-$2}
export DMPDIR=${DMPDIR:-$3}
config_template_dir=${config_template_dir:-$4}
export COM_OBS=${COM_OBS:-$5}

Note that DUMP is exported so it is available for use in bufr2ioda_satwind_amv_goes.json

Script run_bufr2ioda.py has

    # Input parameters
    CDATE = sys.argv[1]
    DUMP = sys.argv[2]
    DMPDIR = sys.argv[3]
    config_template_dir = sys.argv[4]
    COM_OBS = sys.argv[5]

Variable DUMP is locally defined. The value in DUMP is not available to the json processing.

As a test, add a line to set and export DUMP from JGLOBAL_ATM_PREP_IODA_OBS

 # Run relevant script
 EXSCRIPT=${BUFR2IODASH:-${HOMEgfs}/ush/run_bufr2ioda.py}
 export DUMP=$CDUMP
 ${EXSCRIPT} "${PDY}${cyc}" "${CDUMP}" "${DMPDIR}" "${IODAPARM}" "${COM_OBS}/"

With this change prepatmiodaobs successfully ran to completion using run_bufr2ioda.py. The satwnd.abs goes-16 and goes-17 files generated by the python driver script are identical with those generated by the bash driver script. This is expected since the underlying scripts doing the actual work are identical between run_bufr2ioda.py and run_bufr2ioda.sh.

Commit run_bufr2ioda.py to feature/bufr2ioda at fc8ad87.

@RussTreadon-NOAA
Copy link
Contributor Author

Hold off on creating a PR to merge feature/bufr2iodapy into develop until EID and DAD decide how to resolve the CDUMP versus DUMP issue noted above and discussed in g-w PR #1826.

@RussTreadon-NOAA
Copy link
Contributor Author

Given that most of g-w uses CDUMP for the current cycle, opt to replace DUMP with CDUMP in parm/ioda/bufr2ioda/bufr2ioda_satwind_amv_goes.json. Done at 533fe3c.

CoryMartin-NOAA pushed a commit that referenced this issue Sep 6, 2023
* refactor run_bufr2ioda.sh as run_bufr2ioda.py (#599)

* correct pycodestyle errors in run_bufr2ioda.py (#599)

* replace DUMP with CDUMP for cycle_type (#599)

* remove run_bufr2ioda.sh since functionality is now in run_bufr2ioda.py (#599)

* replace DUMP with RUN in bufr2ioda_satwind_amv_goes.json (#599)

* use argparse and gen_bufr_json in run_bufr2ioda.py (#599)

* fix pycodestyle errors in run_bufr2ioda.py (#599)

* clean up run_bufr2ioda.py, add env python3 line to bufr2ioda_satwind_amv_goes.py (#599)

* change bufr2ioda_satwind_amv_goes.py permissions to executable for all users (#599)

* incoporate reviewer suggestions for bufr2ioda python scripts (#599)

* use wxflow rm_p and reorder commands in run_bufr2ioda.py (#599)
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 a pull request may close this issue.

1 participant