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

Combine open PRs for v2.0.7 #92

Merged
merged 47 commits into from
May 31, 2024
Merged

Combine open PRs for v2.0.7 #92

merged 47 commits into from
May 31, 2024

Conversation

Leynse
Copy link
Contributor

@Leynse Leynse commented May 17, 2024

No description provided.

Leynse and others added 30 commits April 4, 2024 18:38
- Note ; had to change definition of gamma, so not anymore mean gamma over current and upwind point > changes gamma and thus source term and IG growth, have to check still whether that is a problem or not
- todo: clean up
…t to 0 everytime snapwave_solver is called, so now determined again

- Code can be cleaned up by doing a H inout definition probably
…vious ee' part

- and result now only little bit different anymore, because of removal of previous loop now?
… as in paper

- Thereby not using H_inc_old and H_ig_old anymore from wave heights of last time step (but implicitly we still do)
… it again at the start of solve_energy_balance2Dstat

- Remove Sxx outside of subroutine 'solve_energy_balance2Dstat', since that is not needed anymore
…ne 'determine_infragravity_source_sink_term' for clarity

- Also makes it easier to experiment puting the source term determination in the ' do iter=1,niter
'-loop
…ior to iteration loop, to fill 'srcsh', based on effectively incident energy from previous SnapWave timestep

- Then depending on new option, 'snapwave_iterative_srcsh=1, is new default', the subroutine is called again to determine srcsh now based on just updated incident wave energy.
- If nr_sweeps = 4, this is slower because subroutine 'determine_infragravity_source_sink_term' is called multiple times
- But, Hm0 IG converges now immediately, instead of after a few timesteps!

- However, to speed up you can with snapwave_iterative_srcsh = 0 still keep old behaviour of only using the initial filling of 'srcsh'. Depends on application whether slower convergence IG wave height is problem
- TODO: check whether slower performance can still be counteracted some way
- ! Update H(k), used in subroutine to calculate relative waterdepth 'gam' > TL: this creates a variability over sweeps/iterations/timesteps since 'gam' changes every time > seems more stable not to do this
- OMP loop in solver did not seem to provide speedup
- OMP loop in subroutine 'determine_infragravity_source_sink_term' made things slower
- Even without these 3 attempts, performance loss of more times calling subroutine 'determine_infragravity_source_sink_term' in iteration loop seems negligible for small model
…g srcsh only for the first sweep (but still every iteration)
This reverts commit 04a8e61.
…estep we get already a good estimate of srcig and thereby Hig, and not just only conservative shoaling from the boundary

- Only done once to prevent changing H(k) > and thereby gam and alphaig > every time, to prevent oscillations
…fference becomes less important with all the netcdf input boundary input types
…obust: alpha=0.50, theta=1.0, adveciont=1, viscosity=1
- Todo: check whether this is best implementation and results in actual random seed if wmrandom = 1
…ve the mean wave direction is actually added, which gives wrong values in hisfile. Turn off for now
- Done as separate variable 't2', because t1 is real*4 while expected t is real*8
Leynse added 11 commits May 17, 2024 16:08
…vel for quadtree simulation - so quadtree infiltration is not important too:

- Move whole viscosity value nuvisc determination to sfincs_domain.f90
   - Now determine nuvisc per grid cell, depending on refinement level (min of dx-dy)!
   - crsgeo true or false is still considered
   - removed option for direct 'nuvisc' input, still possible to scale through 'nuviscdim'
…nput:

- Remove double write cuminf
- add needed 'if (nm>0) ' checks for quadtree max output
- fix debug mode issues with extra (nm > 0) checks
Leynse added 6 commits May 28, 2024 12:31
…e per level, not grid cell, to save computation time in sfincs_momentum
… value of 1.0 that is later divided by 100, but make this 0.01 directly

- This is the 'viscosity per meter grid cell length'
- Note, to prevent issues with old input, I use in sfincs.inp the 'nuvisc' keyword instead of 'nuviscdim', so people with nuviscdim=1 in their older sfincs.inp suddenly do not have a lot of visco, but have the same default
…t, now change it in the netcdf attribute too
Copy link
Contributor Author

@Leynse Leynse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Indivual changes of pulled PRs reviewed by Maarten

@Leynse Leynse merged commit d1e7286 into main May 31, 2024
@Leynse Leynse deleted the combine_open_PRs_for_v2.0.7 branch May 31, 2024 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant