Vectors #199
Vectors #199
Conversation
| @@ -972,7 +997,7 @@ subroutine init_hist (dt) | |||
| "none", c1, c0, & | |||
| ns1, f_aisnap) | |||
|
|
|||
| call define_hist_field(n_trsig,"trsig","N/m^2",tstr2D, tcstr, & | |||
There was a problem hiding this comment.
Is this correct? This looks like it's reverting a PR from last week?
There was a problem hiding this comment.
N/m is correct. This should be okay, although it was already corrected previously.
There was a problem hiding this comment.
I think it was part of Dave's PR #198 but I'd changed it here as well. Since this is identical to the current master I'm not sure why it's showing up as changed.
There was a problem hiding this comment.
I still haven't figured out what git is doing when it shows PR diffs. Sometimes, it seems like it's comparing to master, other times it seems like it's comparing to the branch base. I still find it confusing.
cicecore/shared/ice_constants.F90
Outdated
| @@ -84,7 +84,8 @@ module ice_constants | |||
| p027 = p055*p5, & | |||
| eps04 = 1.0e-4_dbl_kind, & | |||
| eps13 = 1.0e-13_dbl_kind, & | |||
| eps16 = 1.0e-16_dbl_kind | |||
| eps16 = 1.0e-16_dbl_kind, & | |||
| pi = 3.14159265358979323846_dbl_kind | |||
There was a problem hiding this comment.
I'm not in love with hardwiring pi here. It's already hardwired into icepack. What we really need is for pi to be computed at initialization or set by something like cesm shr_const depending on the situation. Both icepack and cice could use the upgrade. For now, we can leave it this way.
There was a problem hiding this comment.
Agree, I was surprised it wasn't in here to be honest. I'll add this as an issue. Maybe I can get to it before the release.
There was a problem hiding this comment.
Where the variables exist in icepack, we have tried to use those in CICE. Both pi and rad_to_deg are available via queries to icepack. That is probably the right way to do it. Where you need rad_to_deg, query it in that subroutine from icepack and get rid of both pi and rad_to_deg definitions in cice. I guess I am going to suggest that change at this point, should have said that earlier.
There was a problem hiding this comment.
Ok, I agree. I would rather do this the right way now. So I should definitely fix it. However, I have an additional question/clarification.
It looks like pi is set in columnphysics/icepack_parameters.F90. Why not in /configuration/driver/icedrv_constants.F90? It looks like we set numerical constants in both places, though the constants don't overlap. But I guess I don't understand why we don't just set them in one place. Am I being dense?
There was a problem hiding this comment.
You're certainly not being dense! The idea with the constants being in two places in Icepack was to delineate the ones that are essential for the Icepack physics (for groups wanting to port it into their own sea ice models) from the ones that are only needed for the driver, e.g. for the forcing. There might be some that are in the wrong place, and if you spot any, make an issue to fix them. Thanks!
|
@eclare108213 The CMIP6 documentation has guidance for speed and vector components, but does not include direction (for wind or sea ice). I added the "coming from" and "going to" for the atmosphere and ice/ocean meta data directions because the conventions are opposite, which has often confused me. So for wind direction a value of 180deg means it's from the south (http://glossary.ametsoc.org/wiki/Wind_direction). But for an ocean current or sea ice if you said the current/ice direction was 180deg it means it's going to the south. Since both direction fields will be in the same history file I want to make sure it's clear to users they use the different conventions. |
|
@apcraig Great. I'm double checking the tests now. Assuming they all pass I'll go ahead and merge later tonight. Let me know if there are any concerns with merges tonight (like last week's). |
|
Second set of tests passed: https://github.com/CICE-Consortium/Test-Results/wiki/cice_by_bran_forks (development branch 'vectors') I've gone through the history files too to verify everything is working properly. |
Adding output variables for vector speed/direction quantities (ice, atm, ocn). I've verified by looking at history file output that these are done correctly. Addresses Issue #58
Developer(s): Alice DuVivier
Please suggest code Pull Request reviewers in the column at right.
Are the code changes bit for bit, different at roundoff level, or more substantial? bfb
Please include the link to test results or paste the summary block from the bottom of the testing output below.
All tests pass. Test results: https://github.com/CICE-Consortium/Test-Results/wiki/cice_by_bran_forks (branch "vectors")
Does this PR create or have dependencies on Icepack or any other models? N
Is the documentation being updated with this PR? N
If not, does the documentation need to be updated separately at a later time? N
Note: "Documentation" includes information on the wiki and .rst files in doc/source/,
which are used to create the online technical docs at https://readthedocs.org/projects/cice-consortium-cice/.