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.

Sign up for GitHub

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

Jump to bottom

Bugfix 1817 point2grid latlon to laton #1835

Merged
merged 2 commits into from
Jun 21, 2021
Merged

Bugfix 1817 point2grid latlon to laton #1835

merged 2 commits into from
Jun 21, 2021

Conversation

hsoh-u
Copy link
Collaborator

@hsoh-u hsoh-u commented Jun 17, 2021 •
edited
Loading

Pull Request Testing

  • Describe testing already performed for these changes:

The problem was caused the input NetCDF has latitude from north to south. The "units" attribute at the latitude variable is not correct. The problem still happens if the "units" attribute is corrected. The problem was computing the offset from x-index and y-index (two-to_one() method).

./point2grid /d1/personal/hsoh/data/Binyu/wrong_latitude_units/VOLCAT_HIMAWARI-8_FLDK_s2020296_050000_v300250_VCB_w167_FLDK_b2020295_204000_g001_pc.nc "latlon 200 200 45 153 0.1 0.1" bezy_2020296_0500_regrided.nc -field 'name="ash_mass_loading"; level="(0,,)";'

OR

/d1/personal/hsoh/git/bugfixes/bugfix_1817_point2grid_latlon_to_laton/MET/met/bin/point2grid /d1/personal/hsoh/data/Binyu/wrong_latitude_units/VOLCAT_HIMAWARI-8_FLDK_s2020296_050000_v300250_VCB_w167_FLDK_b2020295_204000_g001_pc.nc "latlon 200 200 45 153 0.1 0.1" bezy_2020296_0500_regrided.nc -field 'name="ash_mass_loading"; level="(0,,)";'

./plot_data_plane bezy_2020296_0500_regrided.nc test.ps 'name="ash_mass_loading"; level="(,)";'

Note: The plot by ncview for the input NetCDF (VOLCAT_HIMAWARI-8_FLDK_s2020296_050000_v300250_VCB_w167_FLDK_b2020295_204000_g001_pc.nc) and the output netCDF (bezy_2020296_0500_regrided.nc) are not similar. You have to check the lat/lon by moving the mouse at the ncview

  • The ncview of input NetCDF: the latitude is north to south (latitude decreases with ncview from bottom left)

    • Clicking "Inv P" botton will flip vertically (latitude increases with ncview from bottom left)
    • It's rotated about 45 degrees
      • how to know: check latitude/longitude changes by moving the mouse vertically/horizontally).

  • The ncview of output NetCDF by point2grid: the latitude is south to north

  • Recommend testing for the reviewer(s) to perform, including the location of input datasets, and any additional instructions:

  • Do these changes include sufficient documentation updates, ensuring that no errors or warnings exist in the build of the documentation? [Yes]

It was a bug. No additional documentation.
Old MET supports regrid_data_Plane on this input. MET 10.0 beta 4 or later does not allow because the input is not evenly spaced horizontally/vertically.

  • Do these changes include sufficient testing updates? [Yes]

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

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

The unit test "point2grid_2D_time" has the latitude north to south. SO the output should be updated by this change.

<MET_UNIT_TEST_OUT>/point2grid/point2grid_2D_time_west_bering_sea.nc will have different content

  • Please complete this pull request review by [6/22/2021].

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)
    Select: Organization level software support Project or Repository level development cycle Project
    Select: Milestone as the version that will include these changes
  • After submitting the PR, select Linked issues with the original issue number.
  • After the PR is approved, merge your changes. If permissions do not allow this, request that the reviewer do the merge.
  • Close the linked issue and delete your feature or bugfix branch from GitHub.

@hsoh-u hsoh-u linked an issue Jun 17, 2021 that may be closed by this pull request
The output of point2grid is flipped and rotated with lat/lon to lat/lon conversion #1817
Closed
22 tasks
@hsoh-u hsoh-u added this to the MET 10.1.0 milestone Jun 17, 2021
@jprestop
Copy link
Collaborator

@hsoh-u I see that this is a bugfix for Project MET-10.1.0-beta2 and Milestone MET-10.1.0. Will there be a corresponding pull request to fix this in MET-10.0.0?

@hsoh-u hsoh-u mentioned this pull request Jun 18, 2021
Merged
12 tasks
@hsoh-u
Copy link
Collaborator Author

hsoh-u commented Jun 18, 2021

The PR #1837 is created

@jprestop
Copy link
Collaborator

jprestop commented Jun 21, 2021 •
edited
Loading

@hsoh-u There was nothing listed for "recommend testing for the reviewer(s) to perform" in this pull request. But, I thought I would try running the point2grid and plot_data_plane commands that you listed under the "testing already performed" using:

  • the executables from met-10.1.0-beta1 (/d1/projects/MET/MET_releases/met-10.1.0-beta1/bin/)
  • the executables from the bugfix_1817_point2grid_latlon_to_laton branch (/d1/projects/MET/MET_pull_requests/met-10.1.0/met-10.1.0_beta2/bugfix_1817/MET-bugfix_1817_point2grid_latlon_to_laton_into_develop/met/bin/)

and then see what the differences were.

When I run the met-10.1.0-beta1 point2grid with the following command:

/d1/projects/MET/MET_releases/met-10.1.0-beta1/bin/point2grid /d1/personal/hsoh/data/Binyu/wrong_latitude_units/VOLCAT_HIMAWARI-8_FLDK_s2020296_050000_v300250_VC
B_w167_FLDK_b2020295_204000_g001_pc.nc "latlon 200 200 45 153 0.1 0.1" bezy_2020296_0500_regrided.nc -field 'name="ash_mass_loading"; level="(0,*,*)";'

this is the output I get (The difference in the output from the feature branch is enclosed in asterisks):

DEBUG 1: Reading data file: /d1/personal/hsoh/data/Binyu/wrong_latitude_units/VOLCAT_HIMAWARI-8_FLDK_s2020296_050000_v300250_VCB_w167_FLDK_b2020295_204000_g001_pc.nc
DEBUG 1: get_nc_data(NcVar *, double *) type_id: 10 type name: int64
DEBUG 2: Interpolation options: method = UW_MEAN, vld_thresh = 0.5
DEBUG 2: Output grid: Projection: Lat/Lon Nx: 200 Ny: 200 lat_ll: 45.000 lon_ll: -153.000 delta_lat: 0.100 delta_lon: 0.100
DEBUG 1: get_nc_data(NcVar *, double *) type_id: 10 type name: int64
DEBUG 2: 
DEBUG 2: NcCfFile::getData(NcVar *, const LongArray &, DataPlane &) const -> data was flipped to north.
DEBUG 2: Range of data (name="ash_mass_loading"; level="(0,*,*)";)
DEBUG 2: 	input: 0.0217733 to 5.64083	**regridded: 0.0974455 to 3.68398**.
DEBUG 1: Writing output file: bezy_2020296_0500_regrided.nc

When I run with the bugfix_1817_point2grid_latlon_to_laton branch point2grid, with the following command:

/d1/projects/MET/MET_pull_requests/met-10.1.0/met-10.1.0_beta2/bugfix_1817/MET-bugfix_1817_point2grid_latlon_to_laton_into_develop/met/bin/point2grid /d1/persona
l/hsoh/data/Binyu/wrong_latitude_units/VOLCAT_HIMAWARI-8_FLDK_s2020296_050000_v300250_VCB_w167_FLDK_b2020295_204000_g001_pc.nc "latlon 200 200 45 153 0.1 0.1" be
zy_2020296_0500_regrided.nc -field 'name="ash_mass_loading"; level="(0,*,*)";'

this is the output I get (The difference in the output from the beta1 branch is enclosed in asterisks):

DEBUG 1: Reading data file: /d1/personal/hsoh/data/Binyu/wrong_latitude_units/VOLCAT_HIMAWARI-8_FLDK_s2020296_050000_v300250_VCB_w167_FLDK_b2020295_204000_g001_pc.nc
DEBUG 1: get_nc_data(NcVar *, double *) type_id: 10 type name: int64
DEBUG 2: Interpolation options: method = UW_MEAN, vld_thresh = 0.5
DEBUG 2: Output grid: Projection: Lat/Lon Nx: 200 Ny: 200 lat_ll: 45.000 lon_ll: -153.000 delta_lat: 0.100 delta_lon: 0.100
DEBUG 1: get_nc_data(NcVar *, double *) type_id: 10 type name: int64
DEBUG 2: 
DEBUG 2: NcCfFile::getData(NcVar *, const LongArray &, DataPlane &) const -> data was flipped to north.
DEBUG 2: Range of data (name="ash_mass_loading"; level="(0,*,*)";)
DEBUG 2: 	input: 0.0217733 to 5.64083	**regridded: 0.0794586 to 4.32057**.
DEBUG 1: Writing output file: bezy_2020296_0500_regrided.nc

Is that the expected difference in regridded values?

I am working on kiowa and ncview is not in my path:

kiowa:jpresto:/d1/personal/jpresto/MET/met_help/hsoh_pr_bugfix_1817_develop/current> which ncview
ncview: Command not found.

I looked for it in /usr/local/, but couldn't find it there:

kiowa:jpresto:/d1/personal/jpresto/MET/met_help/hsoh_pr_bugfix_1817_develop/current> find /usr/local -name ncview
kiowa:jpresto:/d1/personal/jpresto/MET/met_help/hsoh_pr_bugfix_1817_develop/current>

I have attached the two test.ps (converted to png so that I could upload them) files (named test_beta1.png
and test_bugfix_1817.png).

test_beta1
test_bugfix_1817

Can you please confirm that this is the expected change?

@hsoh-u
Copy link
Collaborator Author

hsoh-u commented Jun 21, 2021

Yes, it's the expected output. There are no differences at the log messages.
The problem was that the regrided data exists at the wrong location.

/usr/local/ncview/bin/ncview

  • "find /usr/local -name ncview" does not find it because "/usr/local" is a softlink
  • "find /usr/local/ -name ncview" will find ncview

jprestop
jprestop approved these changes Jun 21, 2021
View reviewed changes
Copy link
Collaborator

@jprestop jprestop left a comment

Choose a reason for hiding this comment

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

@hsoh-u Thank you for confirming that this is the expected output and thank you for the tip using "find" in /usr/local!
I verified the code changed and Howard confirmed the new output is expected. All the checks passed. I approved this request.

@hsoh-u
Copy link
Collaborator Author

hsoh-u commented Jun 21, 2021

Correction with log message: the mapping (from cells to to cells) are changed. Some regridded data values are changed, too.

@hsoh-u hsoh-u merged commit f5b0555 into develop Jun 21, 2021
@hsoh-u hsoh-u deleted the bugfix_1817_point2grid_latlon_to_laton branch June 21, 2021 21:17
@JohnHalleyGotway JohnHalleyGotway mentioned this pull request Jul 6, 2021
Merged
12 tasks
JohnHalleyGotway added a commit that referenced this pull request Jul 6, 2021
@JohnHalleyGotway @jprestop @hsoh-u
Update develop-ref after #1835. (#1846)
7b3fa26 
* Update install_met_env.kiowa

Commented out items and added text to description

* #1817 Added to_north argument at two_to_one()

* #1817 Calls two_to_one() with false for the third argument, to_north, if latitude moves north to south

* Feature 1819 automation doc warnings (#1836)

* Removing "/lib" that should not be there.

* #1508 checking stat() methoid returns no error before looking at FILE flag

Co-authored-by: jprestop <jpresto@ucar.edu>
Co-authored-by: Howard Soh <hsoh@kiowa.rap.ucar.edu>
Co-authored-by: hsoh-u <hsoh@ucar.edu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jprestop jprestop jprestop approved these changes

@JohnHalleyGotway JohnHalleyGotway Awaiting requested review from JohnHalleyGotway

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
MET 10.1.0
Development

Successfully merging this pull request may close these issues.

The output of point2grid is flipped and rotated with lat/lon to lat/lon conversion
2 participants
@hsoh-u @jprestop