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

Provide namelist option surf_vel_scale in surface_flux_nml to use abso… #30

Merged
merged 4 commits into from
Feb 28, 2019

Conversation

aekiss
Copy link
Contributor

@aekiss aekiss commented Feb 12, 2019

…lute (rather than relative) wind for all surface fluxes. COSIMA/access-om2#137

@russfiedler, @nichannah does this look like it will work? I haven't checked that it will compile...

@russfiedler
Copy link
Contributor

@aekiss No, the namelist variable is private to the surface_flux module. I wouldn't think it would compile. I think it would be better to make the change here
https://github.com/OceansAus/cice5/blob/6f8f1edf1b775b17067a11f6b4642d7cfdb6ca9c/drivers/auscom/surface_flux_mod.F90#L445-L446

Just make an if block to calculate udif as either the difference or -u_atm or -v_atm. Another way would be to multiply U_surf by a 0/1 mask scalar as appropriate.

@aekiss
Copy link
Contributor Author

aekiss commented Feb 12, 2019

Ah yes that was dumb, I was wondering about the scope of the namelist variable.

I'd initially suggested we modify those lines in surface_flux_mod.F90 (445 to 446 in 6f8f1ed, 440-441 in master) but you suggested we set the surface velocities to zero in the atmospheric flux calculation instead
https://github.com/OceansAus/cice5/blob/master/drivers/auscom/cpl_forcing_handler.F90#L976-L977
as this affects all the turbulent fluxes not just the wind stress.

@russfiedler
Copy link
Contributor

Oops, I'll have to delete that comment from the record!

Actually, I think it's much better, in the MOM/CICE setup, to supply the correct surface velocities to the surface_flux module and let it decide what it wants to do with them. The same with CICE. If it wants the ocean velocity then it can choose whether to use it or not. MOM/SIS is a bit different since it uses the data_override facility to do this sort of thing.

@aekiss
Copy link
Contributor Author

aekiss commented Feb 12, 2019

Don't worry, it will disappear from Slack soon enough... unlike my bad coding here.
Does this update look any better?

@russfiedler
Copy link
Contributor

Looks good to me. Having a quick test run (or at least compile) example would be nice. One for the TWG...

@aekiss
Copy link
Contributor Author

aekiss commented Feb 13, 2019

This didn't compile (can't have an IF inside a WHERE) so to avoid the IF I've reimplemented it in terms of a surface velocity scaling factor surf_vel_scale instead of a logical. This now compiles and I'll try out a test run. Any comments on this approach?

@aekiss aekiss changed the title Provide namelist option absolute_wind in surface_flux_nml to use abso… Provide namelist option surf_vel_scale in surface_flux_nml to use abso… Feb 13, 2019
@russfiedler
Copy link
Contributor

I should have spotted that. You need to break the WHERE construct apart and have two branches.

I think a clearer approach would be to to use the logical but set the value of surf_vel_scale in the code. It's trivial but makes it obvious to the user what needs to be set. I don't think there would ever be a reason for fractional values.

@aekiss
Copy link
Contributor Author

aekiss commented Feb 13, 2019

OK how's this? It compiles happily.
I've also updated the documentation on w_atm - I think I got that right.

@russfiedler
Copy link
Contributor

Looks good. Yeah, I think the documentation for w_atm was incorrect. The original should have said relative not absolute.

@aekiss
Copy link
Contributor Author

aekiss commented Feb 13, 2019

Yes, that's what I thought.
OK I'll give it a test run when I get a spare moment.

@aekiss
Copy link
Contributor Author

aekiss commented Feb 28, 2019

I've completed test runs - it seems fine.
@russfiedler or @nichannah could you merge it please?

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.

3 participants