-
Notifications
You must be signed in to change notification settings - Fork 318
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
Add initial sub-setting of regional source grids as part of the mkmapdata process #806
Comments
@bekozi sorry to hit you with two git issues at once... I'm getting an error when I try subsetting with this script (on cheyenne): I saw your comment that subsetting works in serial only (unless I misunderstood).
I have tried pointing to a single-point DST file, a global DST file, as well as a 5x5pt_amazon DST file. Same error... |
@bekozi I got beyond the prev. error, but I'm getting a new one: |
My files are SCRIP while yours are ESMFMESH... |
@slevisconsulting yep makes sense. We have to have SCRIP grid files for gen_domain, but these files for mkmapdata don't have to be. And we are pretty certain that the ESMF unstructured mesh format will be faster. So there is a reason to switch to them. |
@slevisconsulting I'll still take a look at the file. The issue looks metadata-related, and it will be good to understand the error regardless. |
@bekozi I have switched to using ESMFMESH SRC files instead of SCRIP SRC files, and this has helped with the sub-setting. For example this combination of SRC/DST files works: Could you take a look, please? In the above test I use this script: |
@bekozi in case this helps... |
@slevisconsulting I'll take a look. You are correct that wrapped/unwrapped coordinates should be handled. |
@slevisconsulting I pushed a fix to the ocgis master for this. The ESMF Unstructured format did not have a default coordinate system, so I set it to CF-Spherical. Your script runs to completion now. There was a related issue for this (NCPP/ocgis#492). Note, I am still looking into the index remapping for the subset<->global domain. I hope to leverage ESMF directly using arbitrary sequence indices. It will require some feature development for ESMF/ESMPy though. |
Thank you, @bekozi . |
@bekozi I'm encountering a new failure with this script: The second DST file that you'll see in there works, but the first one gives this error when running with the first SRC file: |
@slevisconsulting I'm pretty sure the For the |
Hmm, I don't think so because:
Not the UGRID file, but the UNSTRUCT file. |
So to summarize a bit more clearly: The second DST file works with both SRC files. |
Okay - thanks for the additional explanation. I wasn't quite sure how to reproduce the issues. |
@slevisconsulting I believe the issue is an incorrect |
@bekozi this file has been working for me all along. Sorry for the confusion. To better convey what I'm trying to say, I have split my script into four separate scripts: |
Thanks @slevisconsulting. I was able to reproduce the issue with your new scripts. I think I know what the problem is, and I will get a fix in likely after the holiday. |
@slevisconsulting I pushed a fix for the UNSTRUCT failure. The script now generates a weight file with the spatial subset. Be sure and set From what I can tell, the destination grid has no spatial overlap with the source UGRID file. I added some better error handling to check the subcomm for empty subsets. Do you also think this is the case? |
@bekozi this one now generates the subset file but still doesn't generate a weight file for me. The new error says:
I see what you mean. The lat/lon combination from the dst file does not seem to be present in this source UGRID file. Thank you for pointing that out :-) |
@slevisconsulting Unfortunately I cannot reproduce the error. Could you send me the python -c "import ESMF; print(ESMF.interface.esmfmkfile.ESMFMKFILE)" If they are useful for debugging, here are the two files my script produced:
It may be worthwhile for me to look at the script you are using. I know you've probably sent this to me already... Can you pass along at your convenience? |
I did a I then tried I am using this script: |
Okay, we are using the same files. Does the spatial subset file I posted match the file generated with your script? Also, could you attach the makefile target to this thread? I would like to confirm which ESMF build you are using. It sounds like you are using the most recent ocgis. Thank you! |
Did you set |
Ok, so the spatial subset that you posted is identical to mine. And I'm using NCHUNKS_DST=1, though I got exactly the same error when I set it to 2. Is this what you mean by "makefile target": |
My apologies for the confusion regarding the makefile. I was hoping you could attach the actual file. However, it is easy enough for me to track it down on Cheyenne! It's good news that the spatial subset files are identical. I'll do some more testing on this end with the weight generation. |
I've reproduced the error on Cheyenne. It appears to be platform related. There is some simple Python code to detect if there are more than 1 chunks that is failing (no idea why). When I remove |
Found the issue. I should have spotted it sooner. The run target variable The script creates the weight file now. Most of the other changes were not in the CLI Python file which explains why past patches were picked up by your testing. |
Thank you, @bekozi . |
As far as I can tell, the weight files (aka map_ files) for regional/point destination grids look correct now other than the indexing issue |
This becomes obsolete with PR #1663. So closing as a wontfix. |
This relates to #643 and is dependent on it.
In order for OCGIS to work reasonably for regional grids, a first step of the process needs to run OCGIS to sub-set the grids to just the area needed for the destination grid. This will improve performance of the process, especially in terms of memory.
@bekozi has this as an option in OCGIS, but he needs to get it working to preserve the global indices, because we'll need that for mksurfdata_map. So we are also dependent on that issue in OCGIS (NCPP/ocgis#494).
@negin513 @slevisconsulting @bekozi
The text was updated successfully, but these errors were encountered: