-
Notifications
You must be signed in to change notification settings - Fork 24
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
Comments
@jvigh I'm looking for some advice on units. Here's what the solar time output will look like for the |
Hi John,
I think this is a good question for SWPC. My preference would be decimal
hours, as I think that's more "human-friendly". If you would like, I can
reach out to get their input.
Jonathan
|
Thanks for the input @jvigh. I'll switch to decimal hours, and I do agree that's more "human-friendly". |
… messages for consistent and eliminate the warning about -thresh not being specified becuase its fine to not specify a threshold.
@jvigh and @DanielAdriaansen, good news, adding support for this new
This includes code changes in 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)? |
@jvigh another detail occurred to me that I wanted to ask you about. Shown below are the masks generated by the following 2 commands:
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:
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? |
@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. |
@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. |
… supported in a single run. Still need to update the user's guide.
@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. |
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:
|
…est to demonstrate, and update the mask_type attribute to include the magic string for the gridded data used for data masking.
…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
…ff options in the usage statement and documentation as recommended by @CPKalb.
* 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>
@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! |
@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: |
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
Labels
Milestone and Projects
Define Related Issue(s)
Consider the impact to the other METplus components.
Enhancement Checklist
See the METplus Workflow for details.
Branch name:
feature_<Issue Number>_<Description>
Pull request:
feature <Issue Number> <Description>
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
The text was updated successfully, but these errors were encountered: