Opened 4 years ago
Closed 4 years ago
#2425 closed Bug (fixed)
rhop diagnostic only available if other diagnostics requested
Reported by: | davestorkey | Owned by: | davestorkey |
---|---|---|---|
Priority: | low | Milestone: | |
Component: | DIA | Version: | v4.0.* |
Severity: | minor | Keywords: | |
Cc: | cetlod |
Description
Context
The iom_put call for potential density ('rhop') is in diaar5.F90 and is in a block of code protected by this IF test:
IF( iom_use( 'botpres' ) .OR. iom_use( 'sshthster' ) .OR. iom_use( 'sshsteric' ) ) THEN
So it is only available if one of those other diagnostics is also requested.
Analysis
...
Fix
The potential density is not required for the calculation of the other diagnostics in that IF-block, so it should be moved out of there and protected with its own "IF(iom_use('rhop'))".
Commit History (5)
Changeset | Author | Time | ChangeLog |
---|---|---|---|
13089 | davestorkey | 2020-06-10T13:23:29+02:00 | TRUNK : fix for ticket #2425. |
13087 | davestorkey | 2020-06-10T12:16:00+02:00 | |
13086 | davestorkey | 2020-06-10T11:10:45+02:00 | Delete branch corresponding to #2425. |
13085 | davestorkey | 2020-06-10T10:59:29+02:00 | |
12711 | davestorkey | 2020-04-08T11:15:01+02:00 |
Change History (15)
comment:1 Changed 4 years ago by davestorkey
comment:2 Changed 4 years ago by davestorkey
In 12712:
comment:3 Changed 4 years ago by davestorkey
The existing call to eos calculates both in-situ and potential density and then in-situ density is used in the calcuations for the steric SSH and bottom pressure. So I have modified the solution slightly to put an ELSE clause in the main IF test - see branch.
Should we add an in-situ density diagnostic since the field is available here? (I don't think this diagnostic exists already?)
comment:4 Changed 4 years ago by davestorkey
Branch (rev 12712) tested in GYRE_PISCES and rhop diagnostic is now available (whereas for the standard r4.0-HEAD code the diagnostic is full of zeroes).
comment:5 Changed 4 years ago by mathiot
I agree with the fix and the suggestion to add the in-situ density as diagnostics as it is computed anyway. By the mean time, probably you could change the L_ar5 to l_ar5 to be consistent with the rest of the code and align the
') .OR. &' within the 4 lines of the IF statement.
However, I have a stupid question about the computation of zrhop and zrhd:
In step.F90, you have this line:
CALL eos ( tsn, rhd, rhop, gdept_n(:,:,:) ) ! now in situ density for hpg computation
This is exatly the same line as in diaar5.
As rhop and rhd are public, a better fix (improve performance and less lines of code) we could simply use these variables instead of recomputed it in diaar5 and output rhop and rhd strait into diawri (This will avoid the need to activate diaar5 flag only for density output), isn't it ? Did I miss something ?
comment:6 Changed 4 years ago by davestorkey
In 12727:
comment:7 Changed 4 years ago by davestorkey
I think Pierre is right, so I propose to remove the unnecessary second call to eos from dia_ar5 and use the already-calculated rhd to calculate botpres (instead of zrhd). Then it seems more logical to put the iom_put call for rhop in dia_wri.
Implemented this in the branch (rev 12727) and I get bit-comparable results for the rhop and botpres diagnostics in GYRE_PISCES.
This fix should be applied to r4.0-HEAD and the trunk.
comment:8 Changed 4 years ago by davestorkey
- Cc cetlod added
Christian - are you happy with this change to diaar5.F90?
comment:9 Changed 4 years ago by cetlod
Sorry for the late reply, YES YES I agree with the proposed solution
comment:10 Changed 4 years ago by davestorkey
In 13077:
comment:11 Changed 4 years ago by davestorkey
In 13085:
comment:12 Changed 4 years ago by davestorkey
In 13086:
comment:13 Changed 4 years ago by davestorkey
In 13087:
comment:14 Changed 4 years ago by davestorkey
In 13089:
comment:15 Changed 4 years ago by davestorkey
- Resolution set to fixed
- Status changed from new to closed
Fixed in r4.0-HEAD and the trunk. Closing the ticket.
In 12711: