Skip to content
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

Implement state class #11

Merged
merged 10 commits into from
Oct 23, 2020
Merged

Implement state class #11

merged 10 commits into from
Oct 23, 2020

Conversation

loganchen39
Copy link
Collaborator

@loganchen39 loganchen39 commented Oct 21, 2020

@travissluka I've finished the class State and passed all the tests. please check and merge my branch develop_chen to branch develop. As for the next step,

  1. Once it's been merged, and since the JEDI github location has been changed, I plan to download the OISSTv3 develop branch and re-compile it from the beginning (since there has been a lot of changes).

  2. This is from your comments, "focus on state for now, I think what we should do for increment is to actually move the common methods of state and increment, as well as the the atlasFieldSet variable, into a common Fields base class. You can either do this now, or wait until you have most of State implemented. That way we wont have as much duplicate code between State and Increment". I know that classes of State and Increment have similar content, and it makes sense to me to combine them in some way, but it is related to the STRUCTURE of the program, shall we follow what JEDI's structure? What is your plan and suggestion?

closes #7

@travissluka
Copy link
Member

You never merged in the latest develop branch, you need to do that and resolve the conflicts first

@travissluka travissluka changed the title Develop chen Implement state class Oct 21, 2020
Copy link
Member

@travissluka travissluka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @loganchen39 , sorry that a lot of this seemed like annoying bookkeeping! (it should get more interesting with the subsequent implementation steps). I added some quick comments, but I'll take a closer look once you merge this with the latest develop. Since a lot has changed in oops (including more tests being added) it's possible that your tests will no longer pass after pulling the latest versions of the jedi repositories.

test/testinput/state.yml Outdated Show resolved Hide resolved
test/testinput/state.yml Show resolved Hide resolved
test/testinput/state.yml Outdated Show resolved Hide resolved
src/oisst/State/State.cc Show resolved Hide resolved
src/oisst/State/State.cc Outdated Show resolved Hide resolved
src/oisst/State/State.cc Show resolved Hide resolved
@travissluka
Copy link
Member

I'm not entirely sure, but I think TravisCI is refusing to run the tests until the merge conflicts are resolved

@travissluka
Copy link
Member

@loganchen39 I fixed a data/ Data/ mixup, the TravisCI tests should work now. Once they do I'll approve and merge.

Now that the code base is starting to get bigger, and we might have more things going on simultaneously, I'm going to start adding "to-do" items as issues in github. I can help out from time to time on some of these, so to prevent overlap assign yourself on an issue before starting work so we know who is doing what (I already assigned you for the next step of implementing Increment). WHen you issue a PR, if you put "closes #<issue_number>" the associated issue will be closed when the PR is merged.

@loganchen39
Copy link
Collaborator Author

@travissluka Thanks for your suggestions. I'm actually quite new to this git/github stuff. Previously I learned that I should never code on develop branch, but only on my own branch develop_chen. If there's any change on develop branch and I have "git pull" to merge to my local develop branch, what are the commands to merge branch develop to develop_chen?

Now that my current version of branch develop_chen passed all the test, I'd like merge it to develop branch on github, and then I'll re-download and re-compile all the code since there's been a lot changes, and then I'll work on with your suggestions above, (e.g. change name sst, delete std::cout etc.).

Copy link
Member

@travissluka travissluka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 TravisCI was being slow yesterday for some weird reason, the tests were just sitting in the queue for hours... weird.

@travissluka travissluka merged commit dd4543b into develop Oct 23, 2020
@travissluka travissluka deleted the develop_chen branch October 23, 2020 15:04
@travissluka
Copy link
Member

:+1

@travissluka Thanks for your suggestions. I'm actually quite new to this git/github stuff. Previously I learned that I should never code on develop branch, but only on my own branch develop_chen. If there's any change on develop branch and I have "git pull" to merge to my local develop branch, what are the commands to merge branch develop to develop_chen?

Now that my current version of branch develop_chen passed all the test, I'd like merge it to develop branch on github, and then I'll re-download and re-compile all the code since there's been a lot changes, and then I'll work on with your suggestions above, (e.g. change name sst, delete std::cout etc.).

One thing I would recommend is that you use branches with meaningful names. It's best practice to create a new branch, from develop, for each new feature/bugfix that is being added. Reusing the same branch name can cause small issues with other people pulling your branch.

when you're going back to develop, just run

git checkout develop
git pull

to get the latest develop branch, and make sure to run make update within the build directory to have ecbuild pull the latest versions of the JEDI repositories.

when you're working on your work branch, the easiest way to make sure you have the latest develop merged in is run

git fetch --all
git merge origin/develop

with these commands you don't have to switch branches before merging Alternatively you could merge in the latest develop using

git checkout develop
git pull
git checkout <your branch>
git merge develop

.... but that's more to have to do.

@loganchen39
Copy link
Collaborator Author

@travissluka Thanks a lot, will follow these suggestions.

loganchen39 added a commit that referenced this pull request May 21, 2021
* 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 <travissluka@gmail.com>

* 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 <travissluka@gmail.com>

* 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 <ligang.chen@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Where to store the actual "State" variable with JEDI and Atlas?
2 participants