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

Improve logic of TCPairs wrapper #749

Closed
9 of 21 tasks
georgemccabe opened this issue Jan 6, 2021 · 4 comments · Fixed by #937
Closed
9 of 21 tasks

Improve logic of TCPairs wrapper #749

georgemccabe opened this issue Jan 6, 2021 · 4 comments · Fixed by #937
Assignees
Labels
component: python wrapper priority: blocker Blocker priority: medium Medium Priority reporting: DTC NOAA R2O NOAA Research to Operations DTC Project requestor: NCAR National Center for Atmospheric Research requestor: NOAA/EMC NOAA Environmental Modeling Center required: FOR DEVELOPMENT RELEASE Required to be completed in the development release for the assigned project type: enhancement Improve something that it is currently doing
Milestone

Comments

@georgemccabe
Copy link
Collaborator

georgemccabe commented Jan 6, 2021

There are two main things that could be improved in this wrapper.

  1. Set cyclone to current cyclone if processing individually

Currently the wrapper passes in the full list of cyclones to process from the METplus config for every run. This works fine if the input files are sorted by cyclone since the input file contains only that cyclone. If the input file contains many cyclones and a list of cyclones are specified in the METplus config, the wrapper will loop over these values and process once for each. The same input file is read and the output files contain the cyclone in the filename (if included in the template), but each file will contain all cyclones listed in the config file.

If we are looping over a list of cyclones and processing once for each, the environment variable CYCLONE should only contain the current cyclone being processed, not the full list of cyclones.

  1. * added to output filename if processing all cyclones

If no cyclones are specified in the METplus config, all of the available cyclones will be processed. However, if the cyclone template is not specified in the input file template, the output file will contain * as the cyclone value. If * is set for the cyclone value in this case, it should be replaced with "all" or something similar to avoid created a problematic filename.

Describe the Enhancement

See above

Time Estimate

1 day

Sub-Issues

Consider breaking the enhancement down into sub-issues.

  • Add a checkbox for each sub-issue here.

Relevant Deadlines

None

Funding Source

Define the source of funding and account keys here or state NONE.

Define the Metadata

Assignee

  • Select engineer(s) or no engineer required
  • Select scientist(s) or no scientist required

Labels

  • Select component(s)
  • Select priority
  • Select requestor(s)

Projects and Milestone

  • Review projects and select relevant Repository and Organization ones or add "alert:NEED PROJECT ASSIGNMENT" label
  • Select milestone to next major version milestone or "Future Versions"

Define Related Issue(s)

Consider the impact to the other METplus components.

Enhancement Checklist

See the METplus Workflow for details.

  • Complete the issue definition above, including the Time Estimate and Funding Source.
  • Fork this repository or create a branch of develop.
    Branch name: feature_<Issue Number>_<Description>
  • Complete the development and test your changes.
  • Add/update log messages for easier debugging.
  • Add/update unit tests.
  • Add/update documentation.
  • Push local changes to GitHub.
  • Submit a pull request to merge into develop.
    Pull request: feature <Issue Number> <Description>
  • Define the pull request metadata, as permissions allow.
    Select: Reviewer(s), Project(s), Milestone, and Linked issues
  • Iterate until the reviewer(s) accept and merge your changes.
  • Delete your fork or branch.
  • Close this issue.
@georgemccabe georgemccabe added type: enhancement Improve something that it is currently doing component: python wrapper priority: medium Medium Priority requestor: NOAA/EMC NOAA Environmental Modeling Center requestor: NCAR National Center for Atmospheric Research labels Jan 6, 2021
@georgemccabe georgemccabe added this to the METplus-4.0 milestone Jan 6, 2021
@georgemccabe
Copy link
Collaborator Author

We noticed that there are a few lines of log output between the list of environment variables set for the tc_pairs command run and the command run. The env var list should be found immediately before the command. We should fix this when we refactor this wrapper.

@j-opatz j-opatz self-assigned this Mar 4, 2021
@georgemccabe georgemccabe self-assigned this Mar 4, 2021
@georgemccabe
Copy link
Collaborator Author

In PR dtcenter/MET#1750 the logic to handle genesis files changed to set the STORM_ID to the FOF value (i.e. 2020100700_F000_261N_1101W_FOF). Currently the wrapper assumes that if the user sets the storm ID explicitly that it matches the ATCF format and breaks down the string into year, basin, and cyclone. The wrapper logic should be improved to check if it matches this format and use it if so, otherwise pass through the storm ID to be referenced in filename template tags. The {storm_id} should likely be usable regardless of the format.

@georgemccabe
Copy link
Collaborator Author

Per MET Help ticket 99846, add support for setting consensus list.

@TaraJensen TaraJensen added the required: FOR DEVELOPMENT RELEASE Required to be completed in the development release for the assigned project label May 17, 2021
@georgemccabe
Copy link
Collaborator Author

Lots of progress made in branch feature_749_tc_pairs_improvements to handle proper setting of env vars for cyclone, basin, and storm_id for each iteration instead of passing entire list for each run.

georgemccabe added a commit that referenced this issue May 26, 2021
…s that don't match format - use wildcards to find basin/cyclone if those values can't be extracted from storm ID. Removed logic to skip processing storm_id values that don't match the INIT_BEG year to prevent skipping valid storms if an input file crosses over a year or processing multiple years of data. Updated tests to match new behavior
@georgemccabe georgemccabe linked a pull request May 26, 2021 that will close this issue
12 tasks
@TaraJensen TaraJensen added the reporting: DTC NOAA R2O NOAA Research to Operations DTC Project label Jun 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: python wrapper priority: blocker Blocker priority: medium Medium Priority reporting: DTC NOAA R2O NOAA Research to Operations DTC Project requestor: NCAR National Center for Atmospheric Research requestor: NOAA/EMC NOAA Environmental Modeling Center required: FOR DEVELOPMENT RELEASE Required to be completed in the development release for the assigned project type: enhancement Improve something that it is currently doing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants