search

SHYFEM-model / shyfem

System of HydrodYnamic Finite Element Modules
Other
21 stars 33 forks source link

Coordinate transformations and modules #1

Closed cferrarin closed 9 years ago

cferrarin commented 9 years ago

Created modules:

Modification in all files using cartesian and geographical coordinates (basproj.f, new3di.f, newini.f, shyfem.f, subetsa.f, subnsh.f, subsys.f, subwaves.f)

Deleted unused files (tide.f, tide1.f, tides.h, coords*.h)

Adapted Makefile

georgu commented 9 years ago

I have some comments on this pull request: 1) the fix in newini.f is not good. When we are running in isphe==0 (cartesian), we have to use xgv/ygv to compute the f or beta plane. You always use xgeov which is always geographical coordinates. Please adjust. 2) ok for the changes to the tidal module. But why did you call it tidef, and not simply tide or maybe tides? I would prefer one of these names. 3) I think it is a good idea to have the routines that deal with coordinates and projections in seperate files and not in tide or similar. But I would switch names. The name I have chosen for subcoord.f is misleading. The files in subcoord.f are really all dealing with projection issues. And the routines in subproj.f instead are dealing with the coordinates xgv, xgeov, xcartv. So I would propose to switch the file names and the module names to reflect this. subcoord.f would become subproj.f and viceversa. And the modules coordinates and projection will also switch names.

georgu commented 9 years ago

Bye the way: I already found a way to review the changes in the pull request. It is working pretty well for me. Let's see what happens once I accept the first pull request, and how complicated it gets to pull in all the changes. One more thing: I did not include the files you have sent me some days ago by email. I guess they are now all contained in the current pull request. Right?