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 730 main_v3.1 time_info corrupted #741

Merged
merged 1 commit into from
Dec 21, 2020

Conversation

georgemccabe
Copy link
Collaborator

@georgemccabe georgemccabe commented Dec 21, 2020

The bug occurs when a time_info dictionary is passed back into time_util.ti_calculate. The function checks if init and valid are both set in the input dictionary. If so, it uses the loop_by value to determine which of them to delete from the dictionary so it will be recomputed. The call to del on the input dictionary deleted the 'valid' item from the input dictionary which is still used elsewhere in the code.

To fix in v3.1, copy input to ti_calculate into a new dictionary so that it can't be corrupted

Pull Request Testing

  • Describe testing already performed for these changes:
  • Ran the use case using main_v3.1 and confirmed a TypeError and crash occur:
    /d1/projects/METplus/METplus_Development/bugfix_time_info/METplus.main_v3.1/ush/master_metplus.py -c /d1/projects/METplus/METplus_Development/bugfix_time_info/metplus_final.conf
  • Ran the case again using the fixed version to verify that although there are errors (data is not available), the case runs to completion.
    /d1/projects/METplus/METplus_Development/bugfix_time_info/METplus.main_v3.1_fix/ush/master_metplus.py -c /d1/projects/METplus/METplus_Development/bugfix_time_info/metplus_final.conf
  • Recommend testing for the reviewer to perform, including the location of input datasets:
  • Review the output on kiowa or rerun to verify that the change fixes the crash.
  • Will this PR result in changes to the test suite? [No]

    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.

@georgemccabe georgemccabe added this to the METplus-3.1.1 Bugfix milestone Dec 21, 2020
@georgemccabe georgemccabe linked an issue Dec 21, 2020 that may be closed by this pull request
21 tasks
Copy link
Collaborator

@JohnHalleyGotway JohnHalleyGotway left a comment

Choose a reason for hiding this comment

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

The problem description and fix make sense. I logged onto kiowa and ran the tests that George listed. I was able to replicate the problem and the fixed behavior that he described. I approve.

@JohnHalleyGotway JohnHalleyGotway merged commit 4ae063a into main_v3.1 Dec 21, 2020
@JohnHalleyGotway JohnHalleyGotway deleted the bugfix_740_main_v3.1_time_info_corrupt branch December 21, 2020 21:05
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.

Time info corrupted during GridStat run
2 participants