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

Fix support for int64 NetCDF variable types. #2123

Closed
9 of 22 tasks
JohnHalleyGotway opened this issue Apr 4, 2022 · 4 comments · Fixed by #2127, #2126 or #2128
Closed
9 of 22 tasks

Fix support for int64 NetCDF variable types. #2123

JohnHalleyGotway opened this issue Apr 4, 2022 · 4 comments · Fixed by #2127, #2126 or #2128
Assignees
Labels
MET: Library Code priority: high High Priority requestor: METplus Team METplus Development Team type: bug Fix something that is not working

Comments

@JohnHalleyGotway
Copy link
Collaborator

Describe the Problem

While running plot_data_plane to test out some data for dtcenter/METplus#1551, I ran across some unexpected behavior. I suspect that there's an uninitialized time variable in the vx_data2d_nccf library.

Expected Behavior

When the valid time cannot be parsed from a CF-compliant gridded NetCDF file, it should be reported as unixtime 0, i.e. 19700101. However when I rerun the same plot_data_plane command several times, sometimes it's reported correctly as unixtime 0 and other times, I see a garbage value.

Environment

Describe your runtime environment:
1. Machine: Mac Laptop
2. OS: MacOS
3. Software version number(s): MET version 10.1.0

To Reproduce

Describe the steps to reproduce the behavior:
1. Download obs.txt.gz
obs.nc.gz
2. Run plot_data_plane multiple times with "-v 4" and watch the log messages

plot_data_plane obs.nc obs.ps 'name="precipitation"; level="(*,*)";' -title "Observation precipitation" -v 4

Here's good log output:

DEBUG 4: Data plane information:
DEBUG 4:    plane min: 0
DEBUG 4:    plane max: 2.10354
DEBUG 4:    valid time: 19700101_000000
DEBUG 4:    lead time: 000000
DEBUG 4:    init time: 19700101_000000
DEBUG 4:    accum time: 000000

And here's bad log output:

DEBUG 4: Data plane information:
DEBUG 4:    plane min: 0
DEBUG 4:    plane max: 2.10354
DEBUG 4:    valid time: -21924652902-03_082952
DEBUG 4:    lead time: 000000
DEBUG 4:    init time: -21924652902-03_082952
DEBUG 4:    accum time: 000000

Please inspect the library code and look for uninitialized time variables. Please initialize them to unixtime 0.

Relevant Deadlines

List relevant project deadlines here or state 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: @hsoh-u
  • Select scientist(s) or no scientist required: none needed

Labels

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

Projects and Milestone

  • Select Organization level Project for support of the current coordinated release
  • Select Repository level Project for development toward the next official release or add alert: NEED PROJECT ASSIGNMENT label
  • Select Milestone as the next bugfix version

Define Related Issue(s)

Consider the impact to the other METplus components.

Bugfix 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 main_<Version>.
    Branch name: bugfix_<Issue Number>_main_<Version>_<Description>
  • Fix the bug 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 main_<Version>.
    Pull request: bugfix <Issue Number> main_<Version> <Description>
  • Define the pull request metadata, as permissions allow.
    Select: Reviewer(s) and Linked issues
    Select: Organization level software support Project for the current coordinated release
    Select: Milestone as the next bugfix version
  • Iterate until the reviewer(s) accept and merge your changes.
  • Delete your fork or branch.
  • Complete the steps above to fix the bug on the develop branch.
    Branch name: bugfix_<Issue Number>_develop_<Description>
    Pull request: bugfix <Issue Number> develop <Description>
    Select: Reviewer(s) and Linked issues
    Select: Repository level development cycle Project for the next official release
    Select: Milestone as the next official version
  • Close this issue.
@hsoh-u
Copy link
Collaborator

hsoh-u commented Apr 5, 2022

I cannot reproduce this problem on seneca. Would you add a whole log file with -v 5 or above?

DEBUG 4: NcCfFile::open() -> parsing units for the time variable "seconds since 1970-01-01 00:00:00 UTC"
DEBUG 4: parse_cf_time_string() -> parsed NetCDF CF convention time unit string "seconds since 1970-01-01 00:00:00 UTC"
DEBUG 4:                 as a reference time of 19700101_000000 and 1 second(s) per time step.
DEBUG 4: get_nc_data(NcVar *, double *) add_offset = 0, scale_factor=1, cell_count=1, is_unsigned_value: 0
DEBUG 1: get_nc_data(NcVar *, double *) type_id: 10 type name: int64
DEBUG 4: NcCfFile::open() -> could not extract init time from the "forecast_reference_time" variable.
DEBUG 4: NcCfFile::open() -> could not extract init time from file name.
DEBUG 4: VarInfoFactory::new_var_info() -> created new VarInfo object of type "FileType_NcCF".

@JohnHalleyGotway
Copy link
Collaborator Author

@hsoh-u, while I have this issue easily reproducible on my machine and in front of me, I'll reassign to myself and take a look at it. Thanks for taking a look on seneca.

@JohnHalleyGotway JohnHalleyGotway self-assigned this Apr 5, 2022
JohnHalleyGotway added a commit that referenced this issue Apr 5, 2022
…, reproducible on seneca. Just initialize the newly allocated time_values array with values of -10. PLEASE REMOVE THAT TEST CODE PRIOR TO SUBMITTING A PR TO FIX THIS ISSUE.
@JohnHalleyGotway
Copy link
Collaborator Author

@hsoh-u please try testing again on seneca. However, this time use the newly created bugfix_2123_main_v10.1_nccf_valid_time branch. I modified nccf_file.cc to explicitly initialize newly allocated memory to non-zero values (-10). That should make the error reproducible every time you run it.

I wasn't sure exactly how the time info should be handled in this case, so I'd like to hand it back off to you to figure out.

Please be sure to REMOVE the debug code I've added to nccf_file.cc prior to submitting a PR.

hsoh-u pushed a commit that referenced this issue Apr 5, 2022
hsoh-u pushed a commit that referenced this issue Apr 5, 2022
hsoh-u pushed a commit that referenced this issue Apr 5, 2022
hsoh-u pushed a commit that referenced this issue Apr 6, 2022
hsoh-u added a commit that referenced this issue Apr 6, 2022
hsoh-u added a commit that referenced this issue Apr 6, 2022
hsoh-u pushed a commit that referenced this issue Apr 6, 2022
@JohnHalleyGotway JohnHalleyGotway moved this from To do to In progress in Coordinated METplus-4.1 Support Apr 6, 2022
@JohnHalleyGotway JohnHalleyGotway linked a pull request Apr 6, 2022 that will close this issue
15 tasks
@JohnHalleyGotway JohnHalleyGotway changed the title Fix likely uninitialized time variable in vx_data2d_nccf. Fix support for int64 NetCDF variable types. Apr 6, 2022
@JohnHalleyGotway
Copy link
Collaborator Author

@hsoh-u I modified the title of this issue to better describe the problem/fix.

Repository owner moved this from To Do to Done in MET 11.0.0-beta1 (6/22/22) Apr 6, 2022
Repository owner moved this from In progress to Done in Coordinated METplus-4.1 Support Apr 6, 2022
JohnHalleyGotway added a commit that referenced this issue Apr 7, 2022
JohnHalleyGotway added a commit that referenced this issue Apr 7, 2022
JohnHalleyGotway added a commit that referenced this issue Apr 8, 2022
jprestop added a commit that referenced this issue May 19, 2022
* added logic to manually trigger a workflow via the GitHub… (#2107)

* Bugfix #2102 main_v10.1 initialize modified_hdr_typ (#2108)

* Bugfix #2115 main_v10.1 Rotated LatLon (#2116)

* Per #2123, added debug code to make this error that was unrepoducible, reproducible on seneca. Just initialize the newly allocated time_values array with values of -10. PLEASE REMOVE THAT TEST CODE PRIOR TO SUBMITTING A PR TO FIX THIS ISSUE.

* #2123 Initialize ValidTime if afile to read time bvariable

* #2123 Support int64 data type

* #2123 Support int64 data type

* Bugfix #2118 main_v10.1 grib1_rotll (#2129)

Co-authored-by: George McCabe <[email protected]>

* Bugfix #2106 main_v10.1 gcc (#2134)

* Feature 2141 v10.1.1 (#2144)

* Add testing and docs badges.

* #14 Using literal instead of numbers and make sure no overflow

* #14 Using literal instead of numbers

* #14 Added tmp_buf_size

* #14 Define n_kw_infos first

* #14 Using literal instead of numbers

* Bugfix #2148 main_v10.1 misses (#2149)

Co-authored-by: George McCabe <[email protected]>

* dtcenter/METplus-Internal#14 formatting code

* Feature 2162 v10.1.2 (#2163)

* Per #2162, update versions and notes for the 10.1.2 bugfix release

* Per #2162, update versions and notes for the 10.1.2 bugfix release

* Per #2162, fixing syntax error

* Add "METplus-Internal" before the issue number for clarity

Co-authored-by: johnhg <[email protected]>

Co-authored-by: Julie Prestopnik <[email protected]>
Co-authored-by: johnhg <[email protected]>

Co-authored-by: George McCabe <[email protected]>
Co-authored-by: johnhg <[email protected]>
Co-authored-by: Howard Soh <[email protected]>
Co-authored-by: hsoh-u <[email protected]>
Co-authored-by: Julie Prestopnik <[email protected]>
@JohnHalleyGotway JohnHalleyGotway added the priority: high High Priority label Feb 22, 2023
@JohnHalleyGotway JohnHalleyGotway removed the alert: NEED ACCOUNT KEY Need to assign an account key to this issue label Oct 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
MET: Library Code priority: high High Priority requestor: METplus Team METplus Development Team type: bug Fix something that is not working
Projects
No open projects
2 participants