search

JMMP-Group / SEVERN-SWOT

Severn estuary 500m ocean model
MIT License
1 stars 2 forks source link

Feature/restore severn #21

Closed jpolton closed 3 years ago

jpolton commented 3 years ago

Here are some code improvements and also restoring some things I broke (overwrote with SEAsia config edits). Thank goodness for version control...

In git one makes suggestions for edits by putting them on a separate branch and the creating a 'pull request'. The reviewers can then have a look at the changes ("Files changed") or checkout this branch and try it. Then can then make comments, suggest further changes, reject etc. the request.

When the reviewers are happy someone clicks "Merge Pull request" and the edits go into the master branch.

By working on branches and having reviewers, you limit opportunities to break the master. I should have got you into these habits sooner and avoided my mistake.

micdom commented 3 years ago

@jpolton @mpayopayo I've run everything again to check if this restored version is ok. It is ok and working (unforced and with barotropic tides)!

But, I've noticed that namelist_FES14.bdy has now ln_mask_file = .false. previously I had run it with .true.

Also, when running the barotropic tide run, now it produces only the SEVERN_baroT_1h_t.nc, while previously was producing SEVERN_baroT_1d_t.nc, SEVERN_baroT_1d_u.nc, SEVERN_baroT_1d_v.nc

micdom commented 3 years ago

@jpolton @mpayopayo the run was just too short, it produces also SEVERN_baroT_1d_t.nc, SEVERN_baroT_1d_u.nc, SEVERN_baroT_1d_v.nc

mpayopayo commented 3 years ago

@jpolton @micom There is some inconsistency on the dates we ran, the tides call for 2005 but the model is 2000, I guess for now it doesn't matter.

I also noticed the ln_mask_file=.false. so I don't understand why we create the mask if we don't use it.

I don't have access to sn_src_msk = '/projectsa/NEMO/nibrun/ARCHER_DATA/INPUTS/AMM15/amm15_mask.nc' although that doesn't seem to give any problem

Would it be easier to have all the changes we include in the wiki on the namelist_FES14.bdy already on the file?

jpolton commented 3 years ago

I turned the ln_mask_file = .false. because I released it wasn't needed for these experiments. (@mpayopayo it may yet be needed for time-varying boundary conditions so I wouldn't delete the method for its creation just yet).

@mpayopayo I don't think that the date inconsistency will matter for the tide only forcing but yes that is something to watch when time-varying forcing is used.

@mpayopayo it would make sense to put all the edits into namelist_FES14.bdy so that it can just be run without further edits. The associate wiki text can then either be deleted or left/editted as background information into the structure of the file should someone want to make their own changes (preferred).

@mpayopayo The "proper" way to make edits is to 1) create a new branch from the master. 2) make the edits there. 3) submit a pull request back to the master, so we can see the changes being proposed and all benefit from the updates. Maybe we need to do a tutorial on this as a Monday meeting thing, or something.