-
Notifications
You must be signed in to change notification settings - Fork 48
COSP-RTTOV #127
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
COSP-RTTOV #127
Conversation
Merge updates into working version (2023/01/30)
Merging with the main branch. Should not do again.
The RTTOV v13 code files are copies of cosp_rttov_interface.F90 and cosp_rttov11.F90 No changes made yet.
…t on cosp_rttov_v13
… added conditional dependencies in cosp2_test.F90 and cosp.F90
…efile has changed cosp_rttov_interface_v13.F90 will not be compiled unless RTTOV is being built into COSP, so I no longer need #ifdef RTTOV statements in this file.
…it passes the RTTOV instrument, channel, etc information into the cosp_rttov_init call.
… Allocation added to cosp_rttov_simulate
…in the rttov_interface file.
Shifted DDT in both the STUB and main files.
…nto COSP_RTTOV_PR
|
I've just pulled in the most recent updates, but the standard (non-swathed) ifx tests are now failing. The differences are suspiciously similar to those introduced by the compiler changes here, which makes me think that the kgo tests did not fully update. However, I've checked that those changes have all been brought in correctly. It is possible that the new compiler has broken the bit-for-bit reproducibility with the new PR, but this seems unlikely to me. Am I missing something obvious and do you have suggestions on how to troubleshoot? @dustinswales @RobertPincus @alejandrobodas |
|
I've looked at the md5 hashes of the outputs of the failed test and they don't match the hashes of the v007 KGOs (or at previous versions). This suggests that there is something in this PR that does break bit-comparability for ifx. |
|
It appears that there are several things going on here.
I suspect that the other variables that do not match kgos are experience similar issues. We'll see! |
|
@jshaw35 Although I don't think this will solve the issue I note that particle size WRK arrays are not being masked for thin an clear columns in |
|
@mo-abodas I have brought this PR in line with #143. It should only require v007 KGOs for the new swathed tests. We are so close! |
|
@jshaw35 should I give it a try at uploading the swath outputs from the latest checks as KGOs? |
@alejandrobodas that would be great! |
|
@jshaw35 the file attached contains the updated .out and .md5 files, and the KGOs URLs. |
|
@alejandrobodas all checks are passing! Now all I think this needs is a formal code review. |
alejandrobodas
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jshaw35 I've done a quick review, and my main request is to replace the print statements with calls to errorMessage. Nearly there!
|
@mo-abodas how do things look now? |
|
Thanks @jshaw35 it looks good to me! |
RobertPincus
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @jshaw35 for all this hard work!
|
@alejandrobodas @jshaw35 Looks good, thanks for all this effort. |
|
@alejandrobodas great! When you merge could you also create a new tag to be used with CESM integration? |
|
@jshaw35 PR merged and v2.2.0 created. Thanks again and congratulations! |
|
@jshaw35 Excellent work. Congratulations! |
This PR contains significant additions to COSP that enable two new functionalities:
For an exhaustive description of implementation, validation, and science applications, see our GMD paper (https://gmd.copernicus.org/articles/18/4935/2025/). @dustinswales has been an invaluable contributor to all aspects of this work.
Remaining items: