use abstract route class #405
Conversation
nmizukami
added
cesm-coupling
There was a problem hiding this comment.
I think this is great. A nice improvement that actually decreases the amount of code and removes duplication, which is great. It also allows new methods to be added without touching the old ones which is nice for code management.
I have a few suggestions, mostly to add a few more comments. I also noticed something I didn't notice before which is a module that is different than it's filename. That might be something good to make consistent. But, it's not needed to do here, that could be seperate from this.
route/build/src/accum_runoff.f90
Outdated
| @@ -1,147 +1,64 @@ | |||
| MODULE accum_runoff_module | |||
| MODULE accum_route_module | |||
There was a problem hiding this comment.
I didn't notice this before. But, the internal module name is different than the filename? That would be good to make consistent. This used to be required for the CESM build dependency generator, but it must not be true anymore.
There was a problem hiding this comment.
I will make module name consistent with file name. Mostly both are the same, but some module name is different than the file name (I will correct later)
There was a problem hiding this comment.
I am wondering if I call a module name xxx_mod for xxx_mod.f90 or no "mod" if I put "_mod", I will need to change most of f90 files (I guess that is fine, but wondering what would be best). Right now, it is not consistent. At least, the compiler does not allow the same module name and subroutine name (and so if I put some tag like _mod, it is easier to have different name.
There was a problem hiding this comment.
So for now, I will leave this as and change later after solid decision
Use an abstract class (
base_route) that holds only reach routing interface and allows several specific routine methods to be instantiated by extending this class. This way, the subroutines that loops through all the reaches (i.e.,accum_runoff,irf_route,kwt_route,kw_route,mc_route,dfw_route), located in each routing f90, are removed and consolidated into one common routine (route_networkin main_route.f90).This is just clean up and no answers should be changed.
TO-DO. now if lake is activated (is_lake_sim is set to true) and routing methods other than IRF, the result is likely wrong. For now, if lake is activate, use only 1 for route_opt.