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 gen_vx_mask to support a new local solar time masking region option #2966

Closed
10 of 22 tasks
JohnHalleyGotway opened this issue Sep 6, 2024 · 12 comments · Fixed by #3008
Closed
10 of 22 tasks
Assignees
Labels
MET: Masking priority: high High Priority reporting: NRL METplus Naval Research Laboratory METplus Project requestor: Navy/NRL Naval Research Laboratory requestor: NOAA/SWPC NOAA Space Weather Prediction Center required: FOR DEVELOPMENT RELEASE Required to be completed in the development release for the assigned project type: enhancement Improve something that it is currently doing
Milestone

Comments

@JohnHalleyGotway
Copy link
Collaborator

JohnHalleyGotway commented Sep 6, 2024

Describe the Enhancement

The gen_vx_mask tool already supports solar altitude (solar_alt) and solar azimuth (solar_azi) masking by computing the solar altitude and azimuth values at each grid point for the time defined by the mask_file setting. mask_file may either be set to an explicit time string in YYYYMMDD[_HH[MMSS]] format or to a gridded data file. If set to a gridded data file, the -mask_field command line option specifies the field of data whose valid time should be used. If the -thresh command line option is not used, the raw solar altitude or azimuth value for each grid point will be written to the output. If it is used, the resulting binary mask field will be written. This option is useful when defining a day/night mask.

This issue will enhance gen_vx_mask to support a new option -type solar_time and provide a way to define the beginning/ending solar time to be included in the mask.

Consider also writing up another development issue to add support for mask.solar_time directly in the configuration files for the STAT tools.

Time Estimate

2 days?

Sub-Issues

Consider breaking the enhancement down into sub-issues.
None needed.

Relevant Deadlines

Note that the SWPC 2770493 funding ends in March 2025. So this work should be done during FY25 Q1 (NRL) or FY25 Q2 (SWPC) during the beta1 or beta2 development cycles.

Funding Source

SWPC 2770493
or
NRL METplus 7730022

Define the Metadata

Assignee

  • Select engineer(s) or no engineer required
  • Select scientist(s) or no scientist required

Labels

  • Review default alert labels
  • Select component(s)
  • Select priority
  • Select requestor(s)

Milestone and Projects

  • Select Milestone as a MET-X.Y.Z version, Consider for Next Release, or Backlog of Development Ideas
  • For a MET-X.Y.Z version, select the MET-X.Y.Z Development project

Define Related Issue(s)

Consider the impact to the other METplus components.

Enhancement 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 develop.
    Branch name: feature_<Issue Number>_<Description>
  • Complete the development 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 develop.
    Pull request: feature <Issue Number> <Description>
  • Define the pull request metadata, as permissions allow.
    Select: Reviewer(s) and Development issue
    Select: Milestone as the next official version
    Select: MET-X.Y.Z Development project for development toward the next official release
  • Iterate until the reviewer(s) accept and merge your changes.
  • Delete your fork or branch.
  • Close this issue.
@JohnHalleyGotway JohnHalleyGotway added type: enhancement Improve something that it is currently doing requestor: NOAA/SWPC NOAA Space Weather Prediction Center MET: Masking priority: high High Priority labels Sep 6, 2024
@JohnHalleyGotway JohnHalleyGotway added this to the MET 12.1.0 milestone Sep 6, 2024
@JohnHalleyGotway JohnHalleyGotway added the required: FOR DEVELOPMENT RELEASE Required to be completed in the development release for the assigned project label Sep 6, 2024
@DanielAdriaansen DanielAdriaansen added requestor: Navy/NRL Naval Research Laboratory reporting: NRL METplus Naval Research Laboratory METplus Project labels Oct 30, 2024
@JohnHalleyGotway
Copy link
Collaborator Author

JohnHalleyGotway commented Nov 4, 2024

Gen-Vx-Mask already supports the solar azimuth and altitude types with examples like this:

gen_vx_mask \
$MET_TEST_INPUT/model_data/grib2/gfsanl/gfsanl_4_20120409_1200_000.grb2 \
20120409_06 \
GFS_SOLAR_AZI.nc \
-type solar_azi

Solar azimuth has values from -180 to 180 degrees:
Screenshot 2024-11-04 at 11 07 05 AM

gen_vx_mask \
$MET_TEST_INPUT/model_data/grib2/gfsanl/gfsanl_4_20120409_1200_000.grb2 \
20120409_06 \
GFS_SOLAR_ALT.nc \
-type solar_alt

Solar altitude has values from -90 to 90 degrees:
Screenshot 2024-11-04 at 11 08 30 AM

Adding the solar time option will create a field of data that basically looks like time zones whose value remains constant for each longitude.

@JohnHalleyGotway
Copy link
Collaborator Author

@jvigh I'm looking for some advice on units. Here's what the solar time output will look like for the 20120409_06 timestamp. At 06Z, solar noon is roughly over the Indian Ocean.
Screenshot 2024-11-04 at 12 27 27 PM
This image shows solar time in seconds of the day, where solar noon = 12 * 60 * 60 = 43,200. Do you like defining the units as seconds? Or would it be more convenient to define it in decimal hours? So report solar noon as 12.0 instead of 43,200?

@jvigh
Copy link
Contributor

jvigh commented Nov 4, 2024 via email

@JohnHalleyGotway
Copy link
Collaborator Author

Thanks for the input @jvigh. I'll switch to decimal hours, and I do agree that's more "human-friendly".

JohnHalleyGotway added a commit that referenced this issue Nov 4, 2024
… messages for consistent and eliminate the warning about -thresh not being specified becuase its fine to not specify a threshold.
JohnHalleyGotway added a commit that referenced this issue Nov 4, 2024
@JohnHalleyGotway JohnHalleyGotway moved this from 🟢 Ready to 🏗 In progress in MET-12.1.0 Development Nov 4, 2024
@JohnHalleyGotway
Copy link
Collaborator Author

JohnHalleyGotway commented Nov 4, 2024

@jvigh and @DanielAdriaansen, good news, adding support for this new -type solar_time option for gen_vx_mask was very straightforward. A feature branch that includes these changes can be found and tested in:

seneca:/d1/projects/MET/MET_pull_requests/met-12.1.0/beta1/MET-feature_2966_local_solar_time

This includes code changes in gen_vx_mask to support the new masking option, one new unit test to demonstrate its use, and updates to the documentation. I also took the opportunity to make log messages more consistent, and also add a units attribute to the NetCDF variable output.

I do need to wait until the the met-12.0.0-rc1 release goes out in around a week before merging this into the develop branch since these changes belong in 12.1.0, not 12.0.0.

But @jvigh this code is ready for your testing and for development with dtcenter/METplus#2586 as well as a new METplus use case to demonstrate it.

What's next? Do you want to do some initial testing before I submit a PR? Or should I just submit the PR now (and NOT merge until after the 12.0.0-rc1 release)?

@JohnHalleyGotway
Copy link
Collaborator Author

JohnHalleyGotway commented Nov 4, 2024

@jvigh another detail occurred to me that I wanted to ask you about. Shown below are the masks generated by the following 2 commands:

> gen_vx_mask G004 20241221_15 \
SOLAR_ALT_20241221_15.nc -type solar_alt \
-thresh gt0

> gen_vx_mask G004 20241221_15 \
SOLAR_TIME_20241221_15.nc -type solar_time \
-thresh 'ge11&&le13'
Screenshot 2024-11-04 at 4 15 59 PM

The top shows where the solar altitude is >0, i.e. the sun is above the horizon. The bottom shows the solar time between 11:00am and 1:00pm. Note that in portions of Greenland, while it is around solar noon, the sun is below the horizon.

These 2 masks could be combined, like this:

> gen_vx_mask G004 20241221_15 \
SOLAR_ALT_20241221_15.nc -type solar_alt \
-thresh gt0

> gen_vx_mask SOLAR_ALT_20241221_15.nc 20241221_15 \
SOLAR_ALT_TIME_20241221_15.nc -type solar_time \
-thresh 'ge11&&le13' -intersection
Screenshot 2024-11-04 at 4 20 00 PM

For the purposes of space-weather verification, do we need the ability to easily combine these masks, thresholding BOTH the solar altitude and the solar time? Or is it sufficient to process these fields separately?

@jvigh
Copy link
Contributor

jvigh commented Nov 4, 2024

@JohnHalleyGotway That's a great question. My hunch would be that it would be nice to easily combine the masks, but many times people may wish to process the fields separately. Let me ask SWPC.

@jvigh
Copy link
Contributor

jvigh commented Nov 5, 2024

@JohnHalleyGotway Regarding testing -- unfortunately, I'm about to go out on vacation and then work travel and I won't really have much bandwidth to do any testing between now and Nov 25. However, if you'd like, we could have @CPKalb do the testing and create a basic use case (if she has availability in the next week or two). I'll let her respond as to her availability and bandwidth to undertake something like this.

JohnHalleyGotway added a commit that referenced this issue Nov 5, 2024
… supported in a single run. Still need to update the user's guide.
@CPKalb
Copy link
Contributor

CPKalb commented Nov 5, 2024

@jvigh and @JohnHalleyGotway I have some availability at the end of this week into next week that I could do the testing and use case.

JohnHalleyGotway added a commit that referenced this issue Nov 5, 2024
JohnHalleyGotway added a commit that referenced this issue Nov 5, 2024
@JohnHalleyGotway JohnHalleyGotway linked a pull request Nov 5, 2024 that will close this issue
17 tasks
@JohnHalleyGotway JohnHalleyGotway moved this from 🏗 In progress to 🔎 In review in MET-12.1.0 Development Nov 5, 2024
@JohnHalleyGotway
Copy link
Collaborator Author

JohnHalleyGotway commented Nov 6, 2024

I was able to add logic to make combining multiple similar mask types together in a single run pretty easy. So I've included that functionality in this Pull Request for these changes. This will be useful for other types of masking operations as well, not just for solar time.

Here's a few examples. Previously, all of these would have required multiple runs of Gen-Vx-Mask:

  1. Selecting points near mid-solar day where the sun is above the horizon:
> gen_vx_mask G004 20241221_15 global_midday.nc \
  -type solar_alt,solar_time -thresh 'gt0','ge11&&le13' \
  -intersection
  1. Selecting western hemisphere tropic points:
> gen_vx_mask G004 G004 western_hemi_tropics.nc \
  -type lat,lon -thresh '>=-23.5&&<=23.5','<0' \
  -intersection
  1. Selecting northwest hemisphere land points with freezing surface temperatures:
> gen_vx_mask gfs.grb2 gfs.grb2 nw_hemi_freezing_land.nc \
  -type data,data,lat,lon \
  -mask_field 'name="LAND"; level="L0";' \
  -mask_field 'name="TMP"; level="Z2";' \
  -thresh '==1','<273','>0','<0' \
  -intersection -v 5

Here's the resulting images:
image

JohnHalleyGotway added a commit that referenced this issue Nov 6, 2024
…est to demonstrate, and update the mask_type attribute to include the magic string for the gridded data used for data masking.
@JohnHalleyGotway JohnHalleyGotway assigned CPKalb and unassigned CPKalb Nov 6, 2024
JohnHalleyGotway added a commit that referenced this issue Nov 6, 2024
…here we only write the timing information of the input data to the gen_vx_mask output when no threshold was applied. This should reduce the number of diffs flagged by PR #3008
JohnHalleyGotway pushed a commit that referenced this issue Nov 22, 2024
…ff options in the usage statement and documentation as recommended by @CPKalb.
JohnHalleyGotway added a commit that referenced this issue Nov 23, 2024
* Per #2966, add new solar_time() function to the vx_solar library.

* Per #2966, add support for new solar_time masking type. Also make log messages for consistent and eliminate the warning about -thresh not being specified becuase its fine to not specify a threshold.

* Per #2966, add a units attribute to the output NetCDF mask variable.

* Per #2966, modify solar azimuth and altitude strings to make the log messages align well.

* Per #2966, add gen_vx_mask unit test to demonstrate the solar_time masking type.

* Per #2966, add documentation about the -solar_time option

* Per #2966, reduce SonarQube code smells in gen_vx_mask

* Per #2966, reduce SonarQube findings

* Per #2966, support multiple mask types with the same mask field being supported in a single run. Still need to update the user's guide.

* Per #2966, add UTC

* Per #2966, update gen_vx_mask docs about supporting multiple -type options in a single run

* Per #2966, update logic to fix using data masking twice, add a unit test to demonstrate, and update the mask_type attribute to include the magic string for the gridded data used for data masking.

* Per #2966, adjust the logic slightly to revert to existing behavior where we only write the timing information of the input data to the gen_vx_mask output when no threshold was applied. This should reduce the number of diffs flagged by PR #3008

* Per #2966, update details about the -union, -intersection, and -symdiff options in the usage statement and documentation as recommended by @CPKalb.

---------

Co-authored-by: MET Tools Test Account <met_test@seneca.rap.ucar.edu>
@github-project-automation github-project-automation bot moved this from 🔎 In review to 🏁 Done in MET-12.1.0 Development Nov 23, 2024
@DanielAdriaansen
Copy link
Contributor

@CPKalb @jvigh @JohnHalleyGotway just wanted to follow up regarding a use case mentioned above. Was the use case just to demonstrate the masking? Or was it specific to a SWPC or other use case where it should have been added? I see this was merged but was unsure whether there's still use case work to be done and if so, whether there's a corresponding issue or not. Thanks!

@jvigh
Copy link
Contributor

jvigh commented Dec 12, 2024

@JohnHalleyGotway SWPC would like a use case that demonstrates stratification by LST. I discussed with Dan Adriaansen whether this could be done under the NRL funding, and not very likely there will be time/funding. It's possible we can do this under the IDIQ funding for SWPC, but we want to finish up the specified deliverables first.

I've created a placeholder issue for the development of this use case:
https://github.com/NCAR/SWPC-Realtime-Evaluation/issues/60

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
MET: Masking priority: high High Priority reporting: NRL METplus Naval Research Laboratory METplus Project requestor: Navy/NRL Naval Research Laboratory requestor: NOAA/SWPC NOAA Space Weather Prediction Center required: FOR DEVELOPMENT RELEASE Required to be completed in the development release for the assigned project type: enhancement Improve something that it is currently doing
Projects
Status: 🏁 Done
Development

Successfully merging a pull request may close this issue.

4 participants