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

Enhance TC-Pairs to disable the output of consensus track members #2429

Closed
JiayiPeng-NOAA opened this issue Feb 2, 2023 · 11 comments · Fixed by #2466 or #2451
Closed

Enhance TC-Pairs to disable the output of consensus track members #2429

JiayiPeng-NOAA opened this issue Feb 2, 2023 · 11 comments · Fixed by #2466 or #2451
Assignees
Labels
MET: Tropical Cyclone Tools priority: blocker Blocker priority: bold for release notes Issue should be bolded in release notes requestor: NOAA/EMC NOAA Environmental Modeling Center type: enhancement Improve something that it is currently doing
Milestone

Comments

@JiayiPeng-NOAA
Copy link

Hi John,
I downloaded the tar file from your email and saved on Hera:
/scratch2/NCEPDEV/ensemble/save/Jiayi.Peng/bak/tc_pairs_consensus_example

From the configure file: /scratch2/NCEPDEV/ensemble/save/Jiayi.Peng/bak/tc_pairs_consensus_example/TCPairsConfig_CONSENSUS,
it shows the UK ensemble consensus setting:
name = "UEMN_CONS";
members = [ "UE00", "UE01", "UE02", "UE03", "UE04",
"UE05", "UE06", "UE07", "UE08", "UE09",
"UE10", "UE11", "UE12", "UE13", "UE14",
"UE15", "UE16", "UE17", "UE18", "UE19",
"UE20", "UE21", "UE22", "UE23", "UE24",
"UE25", "UE26", "UE27", "UE28", "UE29",
"UE30", "UE31", "UE32", "UE33", "UE34",
"UE35" ];
required = [];
min_req = 36;

The tc-pair output file (al132020_CONSENSUS.tcst) includes UEMN_CONS and the 36 ensemble members (UE00, UE01,... UE35).
How can I change the configure file (the setting) in order to do the tc-pair for the ensemble mean only (UEMN_CONS)?

To save time and energy, there is no need to do the tc-pair for individual ensemble members (UE00, UE01, etc).
And also, the output (al132020_CONSENSUS.tcst) does not need include the data for (UE00, UE01, etc).

Best regards,
Jiayi

@JiayiPeng-NOAA JiayiPeng-NOAA added type: enhancement Improve something that it is currently doing alert: NEED MORE DEFINITION Not yet actionable, additional definition required alert: NEED ACCOUNT KEY Need to assign an account key to this issue alert: NEED CYCLE ASSIGNMENT Need to assign to a release development cycle labels Feb 2, 2023
@JohnHalleyGotway JohnHalleyGotway changed the title ensemble TC track spread/error cacculation Enhance TC-Pairs to disable the output of consensus track members Feb 2, 2023
@JohnHalleyGotway JohnHalleyGotway added requestor: NOAA/EMC NOAA Environmental Modeling Center priority: blocker Blocker MET: Tropical Cyclone Tools and removed alert: NEED MORE DEFINITION Not yet actionable, additional definition required labels Feb 2, 2023
@JohnHalleyGotway JohnHalleyGotway added this to the MET 11.1.0 milestone Feb 2, 2023
@JohnHalleyGotway
Copy link
Collaborator

Listing @JiayiPeng-NOAA on the issue as the advising scientist.

@sethlinden sethlinden self-assigned this Feb 2, 2023
@JohnHalleyGotway
Copy link
Collaborator

JohnHalleyGotway commented Feb 8, 2023

Recommend adding a new write_members = TRUE/FALSE config option. See this sample TC-Pairs config file.

consensus = [
   {
      name     = "HCCA_CONS";
      members  = [ "AEMI", "GFSI", "CTCI", "DSHP", "EGRI", "EMN2", "EMXI", "HWFI", "LGEM" ];
      required = [];
      min_req  = 8;
      write_members = TRUE; // Default = TRUE
   },
   {
      name     = "GFEX_CONS";
      members  = [ "GFSI", "EMXI" ];
      required = [];
      min_req  = 2;
      write_members = FALSE;
   },

What to do when a member appears in both lists? Which should take precedence... write_members = TRUE or write_members = FALSE?

Recommend letting FALSE take precedence over TRUE so that we keep track of the model names to be EXCLUDED from the output. That makes the logic much easier to implement rather then keeping track of the model names to be included.

Relevant files:

  • Add new entry for write_members in src/basic/vx_config/config_constants.h.
  • Update consensus parsing in src/tools/tc_utils/tc_pairs/tc_pairs_conf_info.cc and store the new boolean in the ConsensusInfo struct.
  • Update the ConsensusInfo struct in src/tools/tc_utils/tc_pairs/tc_pairs_conf_info.h to add this boolean.
  • Make sure the boolean it TRUE by default to maintain backward compatibility.
      Consensus[i].WriteMembers = (*dict)[i]->dict_value()->lookup_bool(conf_key_write_members);
...
      // If required is empty, default to 0
      if(!Consensus[i].WriteMembers) {
         SkipConsensusMembers.add(Consensus[i].Members); // could do this fancier to prevent duplicates
     }

Use StringArray::has(string) to determine if the model name appears in the list to be skipped.

Once we have our StringArray list of SkipConsensusMembers to be skipped, we should throw those tracks away ASAP rather than actually processing them. Update the existing filter_tracks() function to discard any consensus members that should be skipped.

Add a StringArray object to keep track of which models should be omitted from the output across all consensus definitions.

@JohnHalleyGotway JohnHalleyGotway moved this from 📋 Backlog to 🔖 Ready in MET-11.1.0 Development Feb 8, 2023
sethlinden pushed a commit that referenced this issue Feb 14, 2023
sethlinden pushed a commit that referenced this issue Feb 14, 2023
sethlinden pushed a commit that referenced this issue Feb 14, 2023
…then check those boolean values to add to SkipConsensusMembers. SL. ci-skip-all
sethlinden pushed a commit that referenced this issue Feb 16, 2023
…string array SkipConsensusMembers to keep track of members to skip output for. Modified filter_tracks to skip the tracks (consensus members) that are listed in SkipConsensusMembers. SL
@JohnHalleyGotway JohnHalleyGotway moved this from 🔖 Ready to 🏗 In progress in MET-11.1.0 Development Feb 16, 2023
sethlinden pushed a commit that referenced this issue Feb 20, 2023
sethlinden pushed a commit that referenced this issue Feb 21, 2023
JohnHalleyGotway added a commit that referenced this issue Feb 23, 2023
…, making the specification of consensus.write_members optional. If present in the config file, it'll be used. If not, this warning message is printed:

WARNING:
WARNING: TCPairsConfInfo::process_config() -> "consensus.write_members" is missing. Using default value of true.
WARNING:

We chose to do this because it's going into a minor release (v11.1.0).
We do no want to cause v11.0.0 config files to cause an error when used
in v11.1.0.
@JohnHalleyGotway JohnHalleyGotway linked a pull request Feb 24, 2023 that will close this issue
@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in MET-11.1.0 Development Feb 24, 2023
@JohnHalleyGotway JohnHalleyGotway linked a pull request Feb 24, 2023 that will close this issue
15 tasks
@JiayiPeng-NOAA
Copy link
Author

JiayiPeng-NOAA commented Feb 26, 2023 via email

@JohnHalleyGotway JohnHalleyGotway added priority: bold for release notes Issue should be bolded in release notes and removed alert: NEED ACCOUNT KEY Need to assign an account key to this issue alert: NEED CYCLE ASSIGNMENT Need to assign to a release development cycle labels Feb 28, 2023
@JiayiPeng-NOAA
Copy link
Author

JiayiPeng-NOAA commented Mar 1, 2023 via email

@JohnHalleyGotway
Copy link
Collaborator

JohnHalleyGotway commented Mar 1, 2023

Sorry Jiayi, I'd missed this one. The newly added consensus.write_members option in TC-Pairs is doing what you'd want it to do.

In the process_adecks() function, it...

  1. Reads all the input ADeck track data files.
  2. Derives any consensus tracks defined in the configuration file.
  3. Filters tracks to discard any that shouldn't be retained.

When write_members = FALSE, the members tracks are discarded in the filtering step 3 above.

And that occurs PRIOR TO the matching with the Best track data. So TC-Pairs is NOT wasting any extra time processing the individual ensemble member tracks.

@JiayiPeng-NOAA
Copy link
Author

JiayiPeng-NOAA commented Mar 1, 2023 via email

@JohnHalleyGotway
Copy link
Collaborator

JohnHalleyGotway commented Mar 2, 2023 via email

@JiayiPeng-NOAA
Copy link
Author

JiayiPeng-NOAA commented Mar 16, 2023 via email

@JiayiPeng-NOAA
Copy link
Author

JiayiPeng-NOAA commented Mar 16, 2023 via email

@JiayiPeng-NOAA
Copy link
Author

JiayiPeng-NOAA commented Mar 16, 2023 via email

@georgemccabe
Copy link
Collaborator

Hi @JiayiPeng-NOAA , the METplus config variable names should be in all caps, e.g. TC_PAIRS_CONSENSUS1_WRITE_MEMBERS, TC_PAIRS_CONSENSUS2_WRITE_MEMBERS, etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
MET: Tropical Cyclone Tools priority: blocker Blocker priority: bold for release notes Issue should be bolded in release notes requestor: NOAA/EMC NOAA Environmental Modeling Center type: enhancement Improve something that it is currently doing
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants