search

schism-dev / schism

Semi-implicit Cross-scale Hydroscience Integrated System Model (SCHISM)
http://ccrm.vims.edu/schismweb/
Apache License 2.0
78 stars 84 forks source link

Fix for wave forcing output #126

Closed uturuncoglu closed 3 months ago

uturuncoglu commented 4 months ago

This PR aims to fix the issue related to wave forcing output which is implemented recently - https://github.com/schism-dev/schism/commit/c26afb7d3f45d563b372379545c952cc75a1c0c0

uturuncoglu commented 4 months ago

@josephzhang8 The incoming radiation stress components are in N/m but it seems that compute_wave_force_lon divides them to rho0 (1000 kg/m3). Right? I plotted all components after merging output files and looks fine to me but not sure. This test case is not using realistic wind and it is hard to say something about the results.

uturuncoglu commented 3 months ago

@josephzhang8 @platipodium This PR is still open. I just wonder if you have time to look at this in your side or review the changes. Thanks.

josephzhang8 commented 3 months ago

Thx @uturuncoglu for the reminder! The changes are mostly good. I'll need to remove

noutput=noutput+1

b/c these outputs are separate from others by using their own idout* (i.e. not shared with id_out_var). This avoids messing up the outputs that follow USE_WW3. Shall I accept your changes and then make the edits?

uturuncoglu commented 3 months ago

@josephzhang8 Okay. Let me check it by removing in my side. If it works without any issue, I'll push it and let you know.

uturuncoglu commented 3 months ago

@josephzhang8 Sorry for delay. I am working on another issue in SCHSIM. The WW3 test is hanging on Orion and I am trying to fix the issue. One I fix that I'll test without noutput change and let you know. @platipodium was also mentioning about moving compute_wave_force_loncall out of the model to cap. I'll also do that one which might require very minor PR in the NUOPC part of the code. I'll update you soon.

josephzhang8 commented 3 months ago

@uturuncoglu: no worries and we'll wait for u.... thx!

josephzhang8 commented 3 months ago

@uturuncoglu : I removed icount as well. It's not needed (not wrong either). Thx

uturuncoglu commented 3 months ago

@josephzhang8 I was planing to remove compute_wave_force_lon call as well and move it to the cap before you merge but maybe we could do it later. Anyway, I am plaining to close the following ticket https://github.com/oceanmodeling/schism/issues/4 in UFS Coastal side. Thanks for your help.

josephzhang8 commented 3 months ago

Thx @uturuncoglu . Maybe next time u can tell me when you want me to merge so I can wait. Thx.