-
Notifications
You must be signed in to change notification settings - Fork 20
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
Sce branch #101
Sce branch #101
Conversation
Thanks @yiqunyu ! We'll take a look as soon as we can... The first thing I notice is that there's no test case for this, nor are there any unit tests. Can I ask you to at least throw in a regression test so that we can tell when we change and/or break the behaviour! That'll help us look over and test this too (as it'll also give us a PARAM file for using SCE). |
Just some basic questions as I start to look at this. I see the GLOW model is included, do we know what the license used by them is? We will want to make sure it doesn't conflict with ours. Also, is the GLOW model added the same as the release version, or did you have to modify it in any way to make it compatible with RAM-SCB? |
Sure! I will set up the test soon. @drsteve |
The GLOW model is downloaded from HAO: https://www2.hao.ucar.edu/modeling/glow/code. The version I added is the most recent one (0.982). I had to modify the subroutine glowbasic.f90, which is the main program for running the model, to couple with RAM-SCB. I just noted that I missed the Glowlicense.txt and Glow.txt while uploading. I will add them soon together with the test setting. The license information is also on their website as mentioned above, if you'd like to have a quick check. |
One more note :-) I started modifying the GLOW model in 2016, but some newer versions got released in 2017 and 2018. So I incorporated the changes with the newer versions. Should I note the change date as 2016 still or later? |
Looking at 3b of their license (which I am guessing is why you are asking about the date thing) I would say you should put todays date or whatever the date of the last merge request into RAM ends up being. I'm a little concerned about section 8 of their license ("Export Law Assurances. All Software and any technical data delivered under this Agreement are subject to U.S. export control laws and may be subject to export or import regulations in other countries.") But I also don't understand how that can be true since the code is available open source on GitHub. So we probably don't need to worry about it. |
…t is within SCE).
Sorry we haven't finished looking at this yet @yiqunyu, it's a big job and we've both had other pressures on our time when it comes to RAM-SCB work. As there had been some other changes to the master branch I went ahead and rebased the changes onto this branch, so everything was in sync. I've also made a small change to the Makefile so that the SCE test will run. I'm seeing small differences in the output from the reference result you uploaded, so we'll probably need to update those so that the test passes (my guess is that it's the same small platform-dependent changes we see in other tests, generally differences around 1e-4 or less). If @Pheosics can grab the updated branch and run the test ( As soon as there's a path forward on having tests that pass I think I'll merge this. At that time I'll also open an issue to remind us that we have multiple versions of dependent libraries (e.g., IRI) and we should rationalize that. |
- test only runs 10 minutes - reference files were oddly formatted (hard wrap at ~80 cols) - reference log ran through 30 minutes (plus very minor diffs) - reference pressure was taken from minute 15 - update compares pressure at minute 10 and uses the truncated log
Enabled self-consistent electric field: RSCE, as a new option for the Electric Field.
Precipitation flux is firstly calculated, and passed to electric potential solver, in which the Robinson formula is used to obtain height-integrated conductance or the GLOW model is used to calculate height-dependent conductivity. The latter is parallelized. The GLOW model is added in "srcGlow".