EAMxx: cleanup remappers API #6908
Conversation
bartgol
added
BFB
bartgol
force-pushed
the
bartgol/eamxx/remappers-api-cleanup
branch
from
f3b5870 to
8e87eac
Compare
* Do not allow registration of fields via identifier only * Remove distinction between registration and binding phase * Moved some common implementations in the base class * Provide empty impl for do_XYZ methods in base class * Removed DoNothingRemapper (unused and pointless) * Remove remap(bool) method in favor of remap_fwd/remap_bwd
bartgol
force-pushed
the
bartgol/eamxx/remappers-api-cleanup
branch
from
8e87eac to
5fc1d5d
Compare
* Renamed do_XYZ into XYZ_impl (similar to other classes) * Removed impl method for register_field and registration_begin. Derived class peculiar work is done when ALL fields are registered
bartgol
force-pushed
the
bartgol/eamxx/remappers-api-cleanup
branch
from
fd19645 to
92df1fb
Compare
|
@AaronDonahue @tcclevenger tests pass. Youi can review whenever you have time. |
| P3Data::Ptr Factory::create (IC ic, Int ncol, Int nlev) | ||
| { | ||
| P3Data::Ptr ret; | ||
| switch (ic) { | ||
| case mixed: ret = make_mixed(ncol, nlev); break; | ||
| default: | ||
| EKAT_REQUIRE_MSG(false, "Not an IC: " << ic); | ||
| } | ||
| return ret; |
There was a problem hiding this comment.
What is this for? I'm guessing a random warning cleanup?
There was a problem hiding this comment.
Also, I guess I would have a slight preference for
EKAT_ERROR_MSG("Not an IC: " << ic)
but feel free to ignore.
There was a problem hiding this comment.
Ah, I also prever to avoid assert-like macro that hard-code false. If it's ok, I will attach the change to the next PR.
There was a problem hiding this comment.
What is this for? I'm guessing a random warning cleanup?
This is the "fix compiler warning" commit. The error was something like "control reaches the end of a function returninng non-void". Basically the compiler did not see that there was no way to reach the end: you either returned the value or threw an exception. I still don't know why sometimes compilers give this warning.
Alas, "fixing" it was simple, so I did it.
Remove unused interfaces and simplify API as much as possible
This has been on my TODO list for at least 3 years. After talking to @tcclevenger and deciding that an IOPRemapper may simplify the steps toward DataInterpolation integration, I decided to clean up the remappers interface. The main goal was to get rid of the idea that fields can be registered before being allocated. This was needed YEARS ago, when we were trying to handle the remap back/fort to/from the reference grid (the physics grid) directly from the AtmosphereProcess base class. We soon abandoned that idea, but never retracted the remapper interface, making the class overly complicated for NO reason.
A more detailed list of changes:
do_XYZprotected method asXYZ_impl. I figured since that's what we do in the atm procs classes, one naming style was less confusing.do_registration_beginanddo_register_field: derived class specific stuff is done all inregistration_ends_impl