Namelist Refactor#83
Conversation
- Removing NamelistDefaults in the dycore config - Using simplified ndsl.Namelist - Replacing namelist with config in TranslateDycoreFortranData2Py - Updating translate tests with these changes - Modifying workflow tests until namelist refactor is completed
tweaking translate.yaml
| check_negative: bool = NamelistDefaults.check_negative | ||
| layout: Tuple[int, int] = (1, 1) | ||
| grid_type: int = 0 | ||
| u_max: float = 350.0 # max windspeed for dp config |
There was a problem hiding this comment.
Should this (and the other floats) be type hinted as an NDSL defined Float, or is it sufficient for it to be a Python native float?
There was a problem hiding this comment.
This is a good point. I think eventually they should be changed to Float.
However, I wanted to focus on the Namelist split/refactor for these PRs. If more people think that it should be part of this PR, I'll be overruled and will change it.
There was a problem hiding this comment.
@FlorianDeconinck @romanc I'm proposing a change to the wrapper with the Namelists refactoring. Please let me know if you have feedback on this.
| def compute_parallel(self, inputs, communicator): | ||
| namelist = self.namelist | ||
| grid_generator = MetricTerms.from_tile_sizing( | ||
| npx=namelist.npx, | ||
| npy=namelist.npy, | ||
| npx=self.config.npx, | ||
| npy=self.config.npy, | ||
| npz=int(inputs["npz"]), | ||
| communicator=communicator, | ||
| backend=self.stencil_factory.backend, | ||
| grid_type=namelist.grid_type, | ||
| dx_const=namelist.dx_const, | ||
| dy_const=namelist.dy_const, | ||
| deglat=namelist.deglat, | ||
| ) | ||
| grid_type=self.config.grid_type, | ||
| ) # Note: default dx_const, dy_const, deglat |
There was a problem hiding this comment.
@oelbert I found another spot where I wanted some feedback. Does this look okay?
When calling MetricTerms.from_tile_sizing, I removed the explicit setting of dx_const, dy_const, and deglat. These values existed in the old Namelist, but not in the DynamicalCoreConfig.
instead, I'm relying on the defaults in MetricTerms, which were the same values as the old NamelistDefaults.
…o explicitly set to jjuyeonkim fork and branches
2 participants
Description
This is one of multiple PRs to refactor the NDSL's
NamelistandNamelistDefaultsso that submodules have more ownership over configuration parameters and corresponding default values. NDSL Issue#64Changes include the following:
NamelistDefaultsin the dycore config. instead, the config class will store the default values directly.ndsl.NamelistnamelistwithconfiginTranslateDycoreFortranData2PyFixes: NOAA-GFDL/NDSL#64 (refactor that affects Pace, NDSL, PyFV3, and PySHiELD).
How Has This Been Tested?
PyFV3 and PySHiELD translate tests run as expected compared to the
developbranch. Pace unit tests also pass as expected.Checklist: