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

Bugfix 1508 main_v9.1 tc_gen #1524

Merged
merged 2 commits into from
Oct 14, 2020
Merged

Conversation

JohnHalleyGotway
Copy link
Collaborator

@JohnHalleyGotway JohnHalleyGotway commented Oct 14, 2020

Pull Request Testing

This is to solve the intermittent nightly build failures on the main_v9.1 and develop branches. These pop up for tc_gen as well as series_analysis. The problem is the code incorrectly processing data files as if they were ASCII file lists. The new logic is simpler. Assume it's not a file list if...

  • the list has fewer than 10 files that are all missing

  • the list has greater than 10 files where at least 10 are missing

  • Describe testing already performed for these changes:

I am currently testing this on kiowa in:
/d1/projects/MET/MET_pull_requests/met-10.0_beta1/bugfix_1508/MET-bugfix_1508_main_v9.1_tc_gen_into_main_v9.1

This compiles the merged code and then I manually ran bin/unit_test.sh and wrote the log output to kiowa:/d1/projects/MET/MET_pull_requests/met-10.0_beta1/bugfix_1508/unit_test_bugfix_1508_main_v9.1.log

  • Recommend testing for the reviewer to perform, including the location of input datasets:

    Use the attached files to run on kiowa:
touch TG, # Creates a bogus file to break tc_gen
# Will process each word of the genesis file as if it were a file name (64573 of them)
/usr/local/met-9.1/bin/tc_gen \
-v 5 -log run.log \
-genesis genesis.fort.66.suite1.2016010100.txt \
-track aal012016.txt -config TCGenConfig_2016.txt
# Will correctly process 1 genesis file
/d1/projects/MET/MET_pull_requests/met-10.0_beta1/bugfix_1508/MET-bugfix_1508_main_v9.1_tc_gen_into_main_v9.1/met/bin/tc_gen \
-v 5 -log run.log \
-genesis genesis.fort.66.suite1.2016010100.txt \
-track aal012016.txt -config TCGenConfig_2016.txt

Note that the first line of the log file now says:

DEBUG 5: parse_ascii_file_list() -> File "genesis.fort.66.suite1.2016010100.txt" is not an ASCII file list since there are too many missing files.

aal012016.txt
TCGenConfig_2016.txt
genesis.fort.66.suite1.2016010100.txt

  • Will this PR result in changes to the test suite? [No]

The output is unchanged and the NB should stop failing.

If yes, describe the new output and/or changes to the existing output:

  • After merging, should the reviewer DELETE the feature branch from GitHub? [Yes]

Pull Request Checklist

See the METplus Workflow for details.

  • Complete the PR definition above.
  • Ensure the PR title matches the feature or bugfix branch name.
  • Define the PR metadata, as permissions allow.
    Select: Reviewer(s), Project(s), and Milestone
  • After submitting the PR, select Linked Issues with the original issue number.

Copy link
Collaborator

@georgemccabe georgemccabe left a comment

Choose a reason for hiding this comment

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

I ran the tests and verified that the changes in the PR version fixed the issue. Code changes look good to me.

@georgemccabe georgemccabe merged commit d4d00e0 into main_v9.1 Oct 14, 2020
@georgemccabe georgemccabe deleted the bugfix_1508_main_v9.1_tc_gen branch October 14, 2020 18:40
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.

File list parsing logic continues to fail in the Nightly Build for tc_gen.
2 participants