Implement Geometry class #3
Conversation
There was a problem hiding this comment.
The TravisCI tests are failing because of the oisst_coding_norms test. If you run ctest from the main oisst/ directory within your build directory (not oisst/test) you'll see that one additional test
Test project /home/tsluka/work/oisst/oisst.build/oisst
Start 1: oisst_coding_norms
1/2 Test #1: oisst_coding_norms ...............***Failed 0.29 sec
Start 2: test_oisst_geometry
2/2 Test #2: test_oisst_geometry .............. Passed 0.02 sec
It's annoying, but probably for the best to keep that coding norms test in. If you run with verbose output ctest -R norms -V you'll see the complaints it has (mainly annoying little things like excess white space and such)
src/oisst/Geometry/Geometry.cc
Outdated
| Geometry::Geometry(const Geometry & other) : comm_(other.comm_) { | ||
| //util::abor1_cpp("Geometry::Geometry() needs to be implemented.", __FILE__, __LINE__); | ||
|
|
||
| atlasFunctionSpace_.reset(new atlas::functionspace::StructuredColumns(other.atlasFunctionSpace_->grid(), |
There was a problem hiding this comment.
I'm not sure if all this is needed in the copy constructor here, it might work if the unique pointers were changed to shared pointers, and you just copy the pointers instead. Doesn't really matter at this point though, we can worry about that later when it's time to optimize the code
There was a problem hiding this comment.
Yes let's worry about it later, right now what I (and Jim) want is to get an initial result first. Later on we will optimize the code as I get more familiar with JEDI and ATLAS etc.
|
I have made the suggested changes. However there are still 5 "errors" from oisst_coding_norms. |
|
@loganchen39 the coding style JEDI uses is the that from google (I think this https://google.github.io/styleguide/cppguide.html). I never actually pay much attention to memorizing what the standard is, I just clean it up at the end after seeing the complaints from that test. You'll get used to it. Keep this pull request open. Just fix those 5 errors in the norm, push to this same branch, and once those are fixed I'll approve and that "squash and merge" button below will be clickable letting us merge into |
|
@travissluka I fixed those errors in the norm, compiled and test on my local (Cheyenne) machine, and committed to this develop_chen branch. If you think it's ok, you may approve. It's a good start for me to learn this github process. Thanks, |
* remove Model class, no longer needed (#1) * remove makeobs (#2) * remove makeobs.x, no longer needed * remove Model class, no longer needed (#1) * Implement Geometry class (#3) * ctest passed for Geometry class * Modified test CMakeLists.txt * Travis's suggested changes completed by Ligang * Travis's suggested changes completed by Ligang * add dummy serializeable interfaces (#5) * add serializable interfaces to State (#8) * replace boost::shared_ptr with std::shared_ptr (#9) * change jcsda repo locations (#10) * Implement state class (#11) * Temporary implememting class State for Travis to check. * Updated State.cc * Updating class State * Updating class State. * Updating class State. * Updating class State. * Finished class State, passed all test. * Passed all test for class State. * fix binary path Co-authored-by: Travis Sluka <[email protected]> * Finished class Increment. (#17) * Finished class Increment. * Finished class increment. * Updated increment.yml for class increment. * Updated for class Increment. * add test observations (#22) Merged. * Changed float to double for JEDI, read/write netCDF with float for consistency. (#24) * Create Fields base class for Increment/State (#25) * add MPI support (#26) * add missing ModelAux stubs (#27) * Feature/rename (#28) * rename to umdsst * switch to public jedi repo locations * fix travisci * Implement GetValues (#29) * start GetValues * add level(1) to fields * getvalues working? * Merge class LinearGetValues (#31) * Implemented class LinearGetValues, passed all tests. * minor change of a comment. * add hofx_nomodel test (#32) * add hofx_nomodel test * remove use of external OOPS_TRAPFPE env var, deal with that later * Finished bugfix/ConertToCelsius. (#41) * Finished feature class Covariance. (#42) * fix print in getvalues/lineargetvalues (#45) * Finished Feature/dirac.x (#43) * Implementing dirac.x. * Finished feature dirac.x. * Finishing feature/dirac.x. Co-authored-by: Travis Sluka <[email protected]> * Fixed conversion between Celsius and Kelvin. (#46) * Fixed conversion between Celsius and Kelvin. * Finished bugfix/C_K_conversion. * update for ufo::locations change (#47) * Finished Feature/staticbinit.x (#48) * Implementing staticbinit.x * Finished feature/staticbinit.x * Finished feature/staticbinit.x 2nd Time. * keep up with JEDI (#49) * Finished Feature/var.x (#50) * Implementing feature/var.x. * Implementing feature/var.x. * Implementing var.x * Finished feature/var.x. * Finished feature/var.x. * Feature/add mask field (#51) * added landmask.nc * Finished feature/addMaskField. * Finished feature/addMaskField. * Finished feature/addMaskField. * Remove hardcoded 180 and 360. * Finished feature/useLandmask. (#55) * Finished feature/useLandmask. * Finished feature/useLandmask. * Finished feature/useLandmask. * add eckit::Configuration to LinearGetValues constructor (#59) * Add support for horizontally varying correlation lengths (#58) * clean old bump files on test run * manually specify correlation lengths * fix coding norms * fix dependencies of errorcovariance test * add StdDev variable change (#60) * feature/addQC finished (#62) * Finished AddQC, Solved problems with passing vector of pointers of FieldImpl from C to Fortran. * Finishing feature/addQC. * Feature/better landmask (#63) * Test improved landmask. * Finished feature/betterLandmask. * Finished feature/betterLandmask. * update for oops hofx3d (#64) * Misc updates (#65) * convert corr length from gaussian to GC * add "default" var change * update reference answers * fix docker on travisci (#66) * remove GeneralizedDepartures (#67) Approved. * Bugfix/grid issues (#68) * invert lat on netcdf I/O * landmask is explicitly applied to background * flood the test background file * fix landmask/grid in yamls * update reference answers * add cycling script (#69) * Create Model2GeoVaLs class (#70) * fields working for any number of fields * implement Model2GeoVaLs * remove ggmask from Geometry * cleanup * Rossby radius based horizontal correlation lengths (#73) * rossby radius based corr loc, using dummy RR values * kd interpolation in geometry * using rossby radius in covariance * fix tests and coding norms * document interptogeom * add .dat to lfs * fix compile error * support for modules with new ecbuild (#76) * support for module with new ecbuild * use public atlas/fckit * Feature/experiment - Added some initial background of 0.25x0.25 grid. (#77) * Added some 0.25x0.25 files. * Added new initial background for 20120101. * Updated for some initial background of 0.25x025 grid. * Updated initial background of 0.25x0.25 grid. * Updated yml files with correct renamed file names. * Updated to have correct file names. * update most recent stable repo tags Co-authored-by: Logan (Ligang) Chen <[email protected]>
Implemented the OISST Geometry class which passed the ctest. For the analysis grid I'll start by using the atlas::RegularLonLatGrid 1x1 degree "Slat360x180", [xt=(0, 1, 2, ..., 359), yt=(-89.5, -88.5, ..., 89.5)], which is the same as SODA's analysis grid. In the future we'll increase the resolution to quarter degree, 0.25x0.25. Here I also did not use the eckit::Configuration, which I probably should.