search

CFMIP / COSPv2.0

COSP - The CFMIP (Cloud Feedbacks Model Intercomparison Project) Observation Simulator Package
42 stars 38 forks source link

bug fix in OPAQ variables #15

Closed rodrigoguzman-lmd closed 5 years ago

rodrigoguzman-lmd commented 5 years ago

There was a bug in the COSP_OPAQ subroutine of the lidar simulator. Some opaque cloud profiles were not declared as such because the condition to define a fully attenuated lidar signal in an atmospheric layer was mistakenly excluding the 0 value. This has been corrected by changing the condition to ".ge. 0" (greater or equal than 0) instead of ".gt. 0" (greater than 0) where needed. Indeed, unlike for CALIPSO observations, the lidar simulator Scattering Ratio (SR) signal can abruptly be equal to 0 below an opaque cloud without having gone through a value greater than 0 and less than 0.06.

rodrigoguzman-lmd commented 5 years ago

When running regression test from the python script given in COSPv2.0/driver :

[rguzman@loholt2 driver]$ pwd /bdd/CFMIP/workspace/rguzman/COSP_v2.1/COSPv2.0/driver [rguzman@loholt2 driver]$ python test_cosp2Imp.py data/outputs/UKMO/cosp2_output_um.ref.nc data/outputs/UKMO/cosp2_output_um.nc ############################################################################################ Treating relative differences less than 0.0010000000% as insignificant atb532_perp: 0.15 % of values differ, relative range: -1.31e-05 to 1.84e-05 atb532: 0.15 % of values differ, relative range: -1.31e-05 to 1.82e-05 lidarBetaMol532: 0.15 % of values differ, relative range: -1.31e-05 to 1.82e-05 clopaquecalipso: 22.22 % of values differ, relative range: 5.26e-02 to 2.00e+00 clthincalipso: 36.60 % of values differ, relative range: -1.00e+00 to -5.56e-02 clzopaquecalipso: 32.68 % of values differ, relative range: -1.00e+00 to 7.00e+00 clcalipsoopaque: 1.18 % of values differ, relative range: 5.26e-02 to 1.00e+00 clcalipsothin: 3.22 % of values differ, relative range: -1.00e+00 to -5.56e-02 clcalipsozopaque: 1.49 % of values differ, relative range: -1.00e+00 to 6.00e-01 clcalipsoopacity: 9.80 % of values differ, relative range: -1.00e+00 to 1.70e+00 clopaquetemp: 33.33 % of values differ, relative range: -1.00e+00 to 1.12e-01 clthintemp: 35.95 % of values differ, relative range: -4.56e+27 to 7.74e-02 clzopaquetemp: 32.68 % of values differ, relative range: -1.00e+00 to 1.47e-01 clopaquemeanz: 33.33 % of values differ, relative range: -1.00e+00 to 3.25e+00 clthinmeanz: 35.95 % of values differ, relative range: -1.39e+27 to 1.29e+00 clthinemis: 1.96 % of values differ, relative range: -1.46e+30 to -2.27e-01 clopaquemeanzse: 33.33 % of values differ, relative range: -1.00e+00 to 3.25e+00 clthinmeanzse: 35.95 % of values differ, relative range: -1.39e+27 to 1.29e+00 clzopaquecalipsose: 32.68 % of values differ, relative range: -1.00e+00 to 7.00e+00 lidarBetaMol532gr: 0.14 % of values differ, relative range: -1.44e-05 to 1.37e-05 atb532gr: 0.09 % of values differ, relative range: -1.44e-05 to 1.54e-05 lidarBetaMol355: 0.02 % of values differ, relative range: 2.16e-05 to 2.16e-05 atb355: 0.02 % of values differ, relative range: 2.16e-05 to 2.16e-05 dbze94: 0.17 % of values differ, relative range: -1.77e-04 to 1.37e-04 cltmodis: 0.65 % of values differ, relative range: 5.00e-01 to 5.00e-01 tautmodis: 0.65 % of values differ, relative range: -3.17e-01 to -3.17e-01 tauimodis: 0.65 % of values differ, relative range: -1.00e+00 to -1.00e+00 tautlogmodis: 0.65 % of values differ, relative range: -5.31e-01 to -5.31e-01 tauilogmodis: 0.65 % of values differ, relative range: -1.00e+00 to -1.00e+00 reffclimodis: 0.65 % of values differ, relative range: -1.00e+00 to -1.00e+00 pctmodis: 0.65 % of values differ, relative range: -1.93e-01 to -1.93e-01 iwpmodis: 0.65 % of values differ, relative range: -1.00e+00 to -1.00e+00 All other fields match.

rodrigoguzman-lmd commented 5 years ago

Here are the new figures of the OPAQ variables after correcting the bug: OPAQ_variables_figures.pdf

And the scripts to plot these figures: python_scripts.zip

dustinswales commented 5 years ago

@rodrigoguzman-lmd This change looks small and confined only to the lidar simulator, so can you explain the changes in some of the MODIS diagnostics you show when running the regression tests? Maybe you're using an old reference file? Dustin

rodrigoguzman-lmd commented 5 years ago

@dustinswales Before having corrected the OPAQ bug, when running the regression test I got:

[rguzman@loholt2 driver]$ python test_cosp2Imp.py data/outputs/UKMO/cosp2_output_um.ref.nc data/outputs/UKMO/cosp2_output_um.nc ############################################################################################ Treating relative differences less than 0.0010000000% as insignificant atb532_perp: 0.15 % of values differ, relative range: -1.31e-05 to 1.84e-05 atb532: 0.15 % of values differ, relative range: -1.31e-05 to 1.82e-05 lidarBetaMol532: 0.15 % of values differ, relative range: -1.31e-05 to 1.82e-05 lidarBetaMol532gr: 0.14 % of values differ, relative range: -1.44e-05 to 1.37e-05 atb532gr: 0.09 % of values differ, relative range: -1.44e-05 to 1.54e-05 lidarBetaMol355: 0.02 % of values differ, relative range: 2.16e-05 to 2.16e-05 atb355: 0.02 % of values differ, relative range: 2.16e-05 to 2.16e-05 dbze94: 0.17 % of values differ, relative range: -1.77e-04 to 1.37e-04 cltmodis: 0.65 % of values differ, relative range: 5.00e-01 to 5.00e-01 tautmodis: 0.65 % of values differ, relative range: -3.17e-01 to -3.17e-01 tauimodis: 0.65 % of values differ, relative range: -1.00e+00 to -1.00e+00 tautlogmodis: 0.65 % of values differ, relative range: -5.31e-01 to -5.31e-01 tauilogmodis: 0.65 % of values differ, relative range: -1.00e+00 to -1.00e+00 reffclimodis: 0.65 % of values differ, relative range: -1.00e+00 to -1.00e+00 pctmodis: 0.65 % of values differ, relative range: -1.93e-01 to -1.93e-01 iwpmodis: 0.65 % of values differ, relative range: -1.00e+00 to -1.00e+00 All other fields match.

I don't know where those differences (which are not only for the MODIS simulator) come from. Maybe from the environment where the code is compiled and then runned? I cloned the new repository of COSPv2 on October the 2nd if I remember well.

dustinswales commented 5 years ago

@rodrigoguzman-lmd I'm less concerned with the very small differences coming from the lidar and radar diagnostics than I am with the large differences from the MODIS diagnostics. I just cloned the repo and ran the regression tests and I don't see these differences with the MODIS diagnostics.

rodrigoguzman-lmd commented 5 years ago

This is the reference file from my local repository. If there is no difference at all between your reference file and mine, then I guess those differences in the MODIS diagnostics might come from the way the code is compiled and runned in the machine I'm using, right?

cosp2_output_um.ref.nc.zip

rodrigoguzman-lmd commented 5 years ago

Sorry @dustinswales I forgot to put your name on the previous comment...

dustinswales commented 5 years ago

I see no reason not to put this on the trunk. @alejandrobodas Do you agree? Also, a small change like this does not need to be tagged, correct?

alejandrobodas commented 5 years ago

Hi @dustinswales , @rodrigoguzman-lmd . Yes, I see no problem in getting this into the trunk. A couple of suggestions:

1) Rodrigo, please could you delete the !DEBUG comments in the two lines you're modifying? I think we need to take advantadge of modifications to delete comments that are not meaningful/accurate.

2) Since Dustin doesn't see any changes in the MODIS diagnostics, I assume that the changes that Rodrigo sees are are due to compiler/architecture. Rodrigo, please could you run the regression test against reference files created in your system before this modification? i.e. I'd like to see only the changes that occur in your system due to this change.

Rodrigo, which compiler and architecture are you using? Perhaps we could save your reference files in the repository as a second set of reference files.

rodrigoguzman-lmd commented 5 years ago

Hi @dustinswales and @alejandrobodas ,

  1. I have deleted the "!DEBUG" comments, did a commit called "DEBUG comments erased" and I have pushed it to GitHub.

  2. I have run the regression test against a reference file created in my system, and here is the result:

[rguzman@loholt2 driver]$ python test_cosp2Imp.py data/outputs/UKMO/cosp2_output_um.ref.nc data/outputs/UKMO/cosp2_output_um.nc ############################################################################################ Treating relative differences less than 0.0010000000% as insignificant clopaquecalipso: 22.22 % of values differ, relative range: 5.26e-02 to 2.00e+00 clthincalipso: 36.60 % of values differ, relative range: -1.00e+00 to -5.56e-02 clzopaquecalipso: 32.68 % of values differ, relative range: -1.00e+00 to 7.00e+00 clcalipsoopaque: 1.18 % of values differ, relative range: 5.26e-02 to 1.00e+00 clcalipsothin: 3.22 % of values differ, relative range: -1.00e+00 to -5.56e-02 clcalipsozopaque: 1.49 % of values differ, relative range: -1.00e+00 to 6.00e-01 clcalipsoopacity: 9.80 % of values differ, relative range: -1.00e+00 to 1.70e+00 clopaquetemp: 33.33 % of values differ, relative range: -1.00e+00 to 1.12e-01 clthintemp: 35.95 % of values differ, relative range: -4.56e+27 to 7.74e-02 clzopaquetemp: 32.68 % of values differ, relative range: -1.00e+00 to 1.47e-01 clopaquemeanz: 33.33 % of values differ, relative range: -1.00e+00 to 3.25e+00 clthinmeanz: 35.95 % of values differ, relative range: -1.39e+27 to 1.29e+00 clthinemis: 1.96 % of values differ, relative range: -1.46e+30 to -2.27e-01 clopaquemeanzse: 33.33 % of values differ, relative range: -1.00e+00 to 3.25e+00 clthinmeanzse: 35.95 % of values differ, relative range: -1.39e+27 to 1.29e+00 clzopaquecalipsose: 32.68 % of values differ, relative range: -1.00e+00 to 7.00e+00 All other fields match.

The result is what we expected, now only the 16 OPAQ diagnostics show differences (more details on how I did this in the attached pdf file).

I have also attached the two Makefiles I use to compile COSPv2 in my environment. I think this procedure of having a new (or another) reference file created in the local environment to perform the regression test is the simplest way to get rid of these compiler/architecture issues.

Regression_test_local_ref_file.pdf Makefiles.zip

alejandrobodas commented 5 years ago

Hi @rodrigoguzman-lmd , Thanks for doing this, it now shows a clean regression test with changes in the expected diagnostics. Given that Dustin also checked this in his system, I think we can be confident that the differences are due to compiler/architecture and we can merge this to the master branch. @dustinswales since we have been using tags for all changes that go into the master branch, I think we should keep things consistent and do the same with this. Although it is a minimal code change, it does affect a substantial number of diagnostics. Following the suggestions in the CFMIP meeting of better documenting the changes, we can add a description and impact of the changes, something like this, taken from the pull request:

This change fixes a bug in the COSP_OPAQ subroutine of the lidar simulator. Some opaque cloud profiles were not declared as such because the condition to define a fully attenuated lidar signal in an atmospheric layer was mistakenly excluding the 0 value. Indeed, unlike for CALIPSO observations, the lidar simulator Scattering Ratio (SR) signal can abruptly be equal to 0 below an opaque cloud without having gone through a value greater than 0 and less than 0.06.

This change affects the follwing diagnostics: clopaquecalipso, clthincalipso, clzopaquecalipso, clcalipsoopaque, clcalipsothin, clcalipsozopaque, clcalipsoopacity, clopaquetemp, clthintemp, clzopaquetemp, clopaquemeanz, clthinmeanz, clthinemis, clopaquemeanzse, clthinmeanzse, clzopaquecalipsose.

Pull requests: #15

RobertPincus commented 5 years ago

@rodrigoguzman-lmd, @alejandrobodas -

This was quite an interesting experience in that it pointed out that testing protocol is strict enough to catch differences introduced by different compilers, optimization levels, etc.

To me this suggests that working practice ought to be for users to branch from master and run the regression tests before doing any development. If differences are reported they should update their reference files.

I support the merging of these changes on master and the assigning of a new tag.

On Nov 21, 2018, at 6:15 AM, Rodrigo Guzman notifications@github.com wrote:

Hi @dustinswales and @alejandrobodas ,

• I have deleted the "!DEBUG" comments, did a commit called "DEBUG comments erased" and I have pushed it to GitHub.

• I have run the regression test against a reference file created in my system, and here is the result:

[rguzman@loholt2 driver]$ python test_cosp2Imp.py data/outputs/UKMO/cosp2_output_um.ref.nc data/outputs/UKMO/cosp2_output_um.nc ############################################################################################ Treating relative differences less than 0.0010000000% as insignificant clopaquecalipso: 22.22 % of values differ, relative range: 5.26e-02 to 2.00e+00 clthincalipso: 36.60 % of values differ, relative range: -1.00e+00 to -5.56e-02 clzopaquecalipso: 32.68 % of values differ, relative range: -1.00e+00 to 7.00e+00 clcalipsoopaque: 1.18 % of values differ, relative range: 5.26e-02 to 1.00e+00 clcalipsothin: 3.22 % of values differ, relative range: -1.00e+00 to -5.56e-02 clcalipsozopaque: 1.49 % of values differ, relative range: -1.00e+00 to 6.00e-01 clcalipsoopacity: 9.80 % of values differ, relative range: -1.00e+00 to 1.70e+00 clopaquetemp: 33.33 % of values differ, relative range: -1.00e+00 to 1.12e-01 clthintemp: 35.95 % of values differ, relative range: -4.56e+27 to 7.74e-02 clzopaquetemp: 32.68 % of values differ, relative range: -1.00e+00 to 1.47e-01 clopaquemeanz: 33.33 % of values differ, relative range: -1.00e+00 to 3.25e+00 clthinmeanz: 35.95 % of values differ, relative range: -1.39e+27 to 1.29e+00 clthinemis: 1.96 % of values differ, relative range: -1.46e+30 to -2.27e-01 clopaquemeanzse: 33.33 % of values differ, relative range: -1.00e+00 to 3.25e+00 clthinmeanzse: 35.95 % of values differ, relative range: -1.39e+27 to 1.29e+00 clzopaquecalipsose: 32.68 % of values differ, relative range: -1.00e+00 to 7.00e+00 All other fields match.

The result is what we expected, now only the 16 OPAQ diagnostics show differences (more details on how I did this in the attached pdf file).

I have also attached the two Makefiles I use to compile COSPv2 in my environment. I think this procedure of having a new (or another) reference file created in the local environment to perform the regression test is the simplest way to get rid of these compiler/architecture issues.

Regression_test_local_ref_file.pdf Makefiles.zip

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.