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

Add support for MET tc_gen changes in METplus #801

Closed
20 of 21 tasks
DanielAdriaansen opened this issue Feb 9, 2021 · 6 comments · Fixed by #838
Closed
20 of 21 tasks

Add support for MET tc_gen changes in METplus #801

DanielAdriaansen opened this issue Feb 9, 2021 · 6 comments · Fixed by #838
Assignees
Labels
component: use case wrapper priority: high High Priority requestor: University/UIUC University of Illinois, Urbana-Champaign type: new feature Make it do something new
Milestone

Comments

@DanielAdriaansen
Copy link
Contributor

DanielAdriaansen commented Feb 9, 2021

Describe the New Feature

MET tc_gen went through some major changes for MET-10.0.0-beta3. These changes were requested from multiple users including work being done for #630 and #631. The tc_gen_wrapper needs to be updated to accomodate the MET changes so the work for #630 and #631 can be completed.

The changes to MET can be seen here:
dtcenter/MET#1448
dtcenter/MET#1430
dtcenter/MET#1597
dtcenter/MET#1626

Acceptance Testing

List input data types and sources.
Describe tests required for new functionality.

Time Estimate

Estimate the amount of work required here.
Issues should represent approximately 1 to 3 days of work.

Sub-Issues

Consider breaking the new feature down into sub-issues.

  • Add a checkbox for each sub-issue here.

Relevant Deadlines

List relevant project deadlines here or state NONE.

Funding Source

7790901 - as per Kathryn's guidance

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.

New Feature 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.
@JohnHalleyGotway
Copy link
Collaborator

JohnHalleyGotway commented Feb 9, 2021

For reference, the TC-Gen config files for MET versions 9.1 and 10.0.0 are attached:
TCGenConfig_v9.1.txt
TCGenConfig_v10.0.0.txt

The config file changes are listed below:

  • Add "valid_freq" integer.
  • Replace "lead_window" dictionary with "fcst_hr_window" dictionary.
  • Replace "oper_genesis" dictionary with "oper_technique" string.
  • Change default "desc" from "NA" to "ALL".
  • Add "init_inc" and "init_exc" arrays.
  • Add "basin_mask" array.
  • Replace "genesis_window" dictionary with "dev_hit_window".
  • Replace "genesis_radius" integer with "genesis_match_radius" and "dev_hit_radius" integers.
  • Add "ops_hit_tdiff" integer.
  • Add "discard_init_post_genesis_flag" boolean.
  • Add "dev_method_flag" and "ops_method_flag" booleans.
  • Add "output_flag.genmpr" option.
  • Add "nc_pairs_flag" dictionary.
  • Add "valid_minus_genesis_diff_thresh" threshold.
  • Add "best_unique_flag" boolean.
  • Add "basin_file" string.
  • Add "nc_pairs_grid" string.

@georgemccabe
Copy link
Collaborator

Thanks for the info. This should be pretty straightforward to with the new changes to how the wrappers set env vars in MET config files. The changes are in branch feature_768_met_config ( #768 ). I have already verified that the output from the develop branch matches the output from this branch, but we are waiting for @DanielAdriaansen 's changes to the wrapped MET config files and documentation. The changes for this issue would be much easier with those changes available, so we could either 1) Create a PR, approve the changes, and create a separate issue for the MET config and documentation updates or 2) Create a branch off of the feature branch for #768 for this work.

@DanielAdriaansen
Copy link
Contributor Author

Would #780 work? I thought that PR was meant to move this functionality into METplus faster than all of the work in #768.

If not, I could also re-prioritize my work and start on #768 which might be easier than having a separate PR and/or branch from #768.

@georgemccabe
Copy link
Collaborator

#780 is waiting for PR review (#792) by @j-opatz to get the MET_CONFIG_OVERRIDES functionality into develop faster than the other issue. This needs to happen before you can use that feature for your work-around. #768 involves changing the wrappers to set environment variables for MET config files (starting with METPLUS_) in a new and improved way that is much more efficient and easy to add support for new variables. This is what would make this issue much easier to tackle.

@DanielAdriaansen
Copy link
Contributor Author

I will start work ASAP on #768. Probably day or two I would imagine? Seems to make the most sense to me.

@georgemccabe
Copy link
Collaborator

georgemccabe commented Mar 10, 2021

I have made the first pass at refactoring the wrapper. I still need to:

  • Update the parm/met_config/TCGen_wrapped file to use the new environment variables
  • Update the parm/use_cases/met_tool_wrapper/TCGen use case to use the new METplus config variables
  • Add unit tests for TCGen wrapper and add more tests for new MET config handling functions
  • Update the documentation to include the new config variables in the glossary and python wrappers section

@georgemccabe georgemccabe linked a pull request Mar 11, 2021 that will close this issue
10 tasks
@georgemccabe georgemccabe moved this from In progress to Pull Request Review in METplus-4.0.0-beta5 (5/3/21) Mar 11, 2021
@TaraJensen TaraJensen removed the alert: NEED ACCOUNT KEY Need to assign an account key to this issue label Mar 16, 2021
METplus-4.0.0-beta5 (5/3/21) automation moved this from Pull Request Review to Done Mar 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: use case wrapper priority: high High Priority requestor: University/UIUC University of Illinois, Urbana-Champaign type: new feature Make it do something new
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

4 participants