-
Notifications
You must be signed in to change notification settings - Fork 6
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
Handle Missing Radius of Last Closed Isobar in SCHISM-PaHM #29
Comments
@SorooshMani-NOAA It failed with this message after the
|
Let me try to run it on my side and see what I can find out. |
I tried running SCHISM using the same binaries as @FariborzDaneshvar-NOAA, and the same setup (symlinked all files) and it went through successfully. Maybe the issue had something to do with the platform cloud instance failure. In any case I have the output and still I get 0 winds:
@pvelissariou1 we will provide the Sandy track (generated using the same method) so that you can test. In the meantime do you have any suggestion for us to debug? I used to compile and run SCHISM on PW without issues using this script and the binaries I ran in this case are generated using the same old script (no container, etc.) Do you suggest that I debug it? If so, what flags should I use to debug and where to put breakpoints? We're compiling using the following modules on PW:
and using the commit
|
@FariborzDaneshvar-NOAA and @SorooshMani-NOAA I agree, the error messages might be related to cloud instance issues as Soroosh replied. The modules: |
Let me get the same SCHISM commit and check it out. @SorooshMani-NOAA debuging SCHISM which runs without errors will be a trial and error approach, let me check the particular commit you are using and I'll let you know what I find. |
@FariborzDaneshvar-NOAA @SorooshMani-NOAA I guess you use nws=-1 in the namelist file and you define USE_PAHM=ON when compiling SCHISM. |
Yes, we compiled with |
I checked in detail SCHISM for PaHM related codes in (a) CoastalApp/SCHISM/schim (commit bb616ded), (b) schism-dev/schism (commit ddf15649) and (c) schism-dev/schism master (commit 9517c51e) and found no major differences in PaHM implementation. So, in principle the "fake" track files should work as they do with PaHM and with PaHM+ADCIRC in CoastalApp. I am setting up explicit tests in CoastalApp-testsuite using (a) SCHISM standalone (PaHM activated) and (b) PAHM+SCHISM coupled to check things out. |
Thanks for the update! |
@FariborzDaneshvar-NOAA , @SorooshMani-NOAA , @saeed-moghimi-noaa Background information:I run schism (standalone) using commit ddf1564 from https://github.com/schism-dev/schism for both the BEST and OFCL tracks for hurricane Sandy and the shinnecock inlet case.
The simulation results for the shinnecock.sch case are in hera at:
folders. The data are written in the schout_000001_1.nc files contained in each folder. The BEST simulation results contain non-zero wind speeds that are non-zero as expected. The OFCL simulation results contain only zero wind speeds as we have already discussed. IMPORTANT: Keep in mind, that if the storm path (eye in particular) is not near on inside the computational domain, PaHM produces zero winds (as expected). Also zero winds are produced if there are no data in the track file. Issue resolution:In SCHISM, the GAHM model has been modified slightly to use the radius of the last closed isobar (RRP) to reduce the amount of calculations in the domain (similar to the Holland model) by eliminating the nodal points outside RRP (I disagree with this approach but this is for future discussion with the SCHISM developers). In our case, in the OFCL track files all the RRPs are set to zero, hence the problem with the OFCL files. My suggestion for a temporary workaround is to replace RRP by the max(radius1, radius2, radius3, radius4) of the 34 isotach. Also we might modify SCHISM to use either RRP (if found), or the max R34 found above or setting a default value. |
Hi @pvelissariou1 thanks for testing. I can modify my script and later the |
@SorooshMani-NOAA , @FariborzDaneshvar-NOAA At this point it will be better to comment out the code
blocks in SCHISM for RRP and have a working version for your purposes.
Let's talk about this in the meeting today.
Takis
Panagiotis Velissariou, Ph.D., P.E.
UCAR Scientist
National Ocean and Atmospheric Administration
National Ocean Service
Office of Coast Survey CSDL/CMMB
Physical Scientist - Project Lead
cell: (205) 227-9141
email: ***@***.***
…On Mon, Aug 14, 2023 at 7:17 AM Soroosh Mani ***@***.***> wrote:
Hi @pvelissariou1 <https://github.com/pvelissariou1> thanks for testing.
I can modify my script and later the stormevents code to update the RRP
field for now.
—
Reply to this email directly, view it on GitHub
<#29 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/APC7TP5Q2A3OQ45LBN4CO53XVIJMLANCNFSM6AAAAAA3LRJ3ZA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
As we discussed, in short term we use a fork of SCHISM with updated PaHM code to ignore RRP, and then later we'll decide how to address this during normalization in I'll also rename this ticket to reflect the main issue we're discussing here |
@SorooshMani-NOAA thanks for updating the image. Looks like it worked and runs for Here is also the maximum horizontal wind speed for the Directory of new runs on the NHC_COLAB_2 cluster are:
With this fix, linked issue posted on the ondemand-storm-workflow repository will be resolved! |
@SorooshMani-NOAA , @FariborzDaneshvar-NOAA , @saeed-moghimi-noaa I came up with a solution that seems to work pretty well. This solution will be implemented in PaHM and in SCHISM/PaHM and most likely I'll push it to ADCIRC as well. @SorooshMani-NOAA , Soroosh you might want to implement this solution from your side as well. See the image below: |
@pvelissariou1, in a separate ticket related to NHC collaboration I brought up:
as you've asked me to. @WPringle suggested:
I wanted to follow up with you to see what you think. |
@FariborzDaneshvar-NOAA @SorooshMani-NOAA Let's keep the setup we have right now in SCHISM/PaHM for RRP (GaHM model) where the particular piece of code is commented out and RRP is not used. Very soon I will implement the solution we have for RRP. The SCHISM developers added the RRP code in GaHM to reduce the computational load by excluding the nodal locations where the winds are actually zero or very close to zero. As @WPringle pointed out the fields are reduced eventually to zero at locations outside the RRP having though no physical meaning at these locations. The SCHISM developers will still like to have the RRP code for the reason described above. |
@pvelissariou1 @SorooshMani-NOAA should we close this ticket? |
@FariborzDaneshvar-NOAA @SorooshMani-NOAA Please, let's close it end of next week |
@pvelissariou1 can you please update this ticket and let me know if I can close it? thanks |
After the coupled simulations with PAHM are complete and evaluate the PAHM results I'll push the updates upstream to PaHM and to SCHISM. There is nothing else to add at this moment. Need to update this ticket when the changes to SCHISM/PAHM have been accepted, let's keep it open for 2-3 weeks. |
@FariborzDaneshvar-NOAA , @SorooshMani-NOAA I have updated PaHM to include the RRP resolution for both Holland and GAHM models. There are some rare occassions that all R34 and RRP radii are missing from the track file, and in these cases PaHM reverts to use all the nodal points when performing its interpolations. Later today (01/08/2024), I will push the PaHM updates to SCHISM, I'll let you know. Before I submit a PR, please consider testing the changes to SCHISM by cloning the "cmmb" branch. |
... that is: |
@pvelissariou1 @FariborzDaneshvar-NOAA seems like we can close this? |
Yes, it is done from my side and Fariborz, Soroosh have tested the PaHM updates. Will reopen if needed. |
Use
/lustre/scripts/schism.sbatch
to run SCHISM for the non-perturbed (original) fakedBEST
track that was generated by theondemand-storm-workflow
Directory:
/lustre/hurricanes/florence_2018_Fariborz_OFCL_10_v2/setup/ensemble.dir/runs/original/
The text was updated successfully, but these errors were encountered: