E3SM-Project / polaris

Testing and analysis for OMEGA, MPAS-Ocean, MALI and MPAS-Seaice
BSD 3-Clause "New" or "Revised" License
6 stars 13 forks source link

Add shared single layer yaml file #57

Closed sbrus89 closed 1 year ago

sbrus89 commented 1 year ago

This adds a yaml file for MPAS-Ocean config settings relevant to single layer test cases.

Checklist

sbrus89 commented 1 year ago

@cbegeman, let me know what you think of these when you get a chance.

philipwjones commented 1 year ago

Not sure how you want to manage the transition from MPAS to Omega, but in Omega, we'll be eliminating the config_ prefix and using mixed case rather than underscores.

xylar commented 1 year ago

@philipwjones, do you have an update mock-up of the yaml file we can work from?

Our plan is to use the OMEGA names for the config options exactly as OMEGA expects them and to keep a file that maps from the OMEGA names to the MPAS-Ocean names. We can get that started right after the Hackathon as long as it's pretty clear what the OMEGA conventions are.

And you certainly don't have to have the final OMEGA names or any config options, we just want the current best guess.

dengwirda commented 1 year ago

@sbrus89 and all, I don't think that config_disable_vel_vmix: true, unless there's a change to the way the implicit drag schemes will be handled?

philipwjones commented 1 year ago

@xylar we haven't gotten to specific designs for tendency terms yet, so don't have an update on what a final config file or the variable names are going to be, but can say a few things. First as I mentioned above, remove the config_ prefixes and replace underscores with mixed case. We can match what we want the variable name to be within the code and shorter is better, so TimeIntegration, timeMethod, PressGrad, pGradMethod maybe? Second, each tendency term should own its own on/off switch rather than having a debug group, so these should be distributed into the appropriate tendency configuration group. I don't know what everyone's preference is for the naming of on/off switches (useTend vs tendOn or tendOff) but we should make it consistent. Finally, I also don't know how people feel about introducing additional levels of hierarchy (an additional level of indent). For some tendencies, it might make sense. For example, you could have a parent Vmix (or VertMix?) group with some general vars (main on/off switch) and then sub-groupings to manage the specific variables for supported options (shear, convection, KPP). Tracers are another example where that might make sense - a parent tracer group with sub-groups for the various tracer combinations.

I realize all of that makes a translator much more difficult, but those are the things we'll be thinking about as we move forward with the designs. YAML allows us more flexibility than namelists so we can think about what makes more sense for Omega

xylar commented 1 year ago

@philipwjones, that all sounds fine to me.

we haven't gotten to specific designs for tendency terms yet, so don't have an update on what a final config file or the variable names are going to be, but can say a few things.

No problem, then we'll stick with the MPAS-Ocean names for now but be prepared to switch to OMEGA names as they become clearer.

First as I mentioned above, remove the config_ prefixes and replace underscores with mixed case. We can match what we want the variable name to be within the code and shorter is better, so TimeIntegration, timeMethod, PressGrad, pGradMethod maybe?

Sounds good!

Second, each tendency term should own its own on/off switch rather than having a debug group, so these should be distributed into the appropriate tendency configuration group. I don't know what everyone's preference is for the naming of on/off switches (useTend vs tendOn or tendOff) but we should make it consistent.

Yes, I agree. We don't necessarily use these flags for debugging so they belong in their own groups. My slight preference would be for useTend = true or useTend = false.

Finally, I also don't know how people feel about introducing additional levels of hierarchy (an additional level of indent). For some tendencies, it might make sense. For example, you could have a parent Vmix (or VertMix?) group with some general vars (main on/off switch) and then sub-groupings to manage the specific variables for supported options (shear, convection, KPP). Tracers are another example where that might make sense - a parent tracer group with sub-groups for the various tracer combinations.

I'm very much in favor of this. It won't complicate the translation back to MPAS-Ocean at all, I think. It will make things clearer to users and developers. I know that @cbegeman was also in favor from earlier conversations. Basically, the more info we can provide (by grouping things into smaller groups), while not ballooning the size of the YAML file, the better.

I realize all of that makes a translator much more difficult, but those are the things we'll be thinking about as we move forward with the designs. YAML allows us more flexibility than namelists so we can think about what makes more sense for Omega

I don't think any of this sounds any more complicated than I was expecting. One thing I haven't thought through in terms of translation is exactly how to handle situations where the values on the Omega vs. MPAS-Ocean sides have a different type (e.g. string vs. boolean) or when one config option on one side maps to multiple on the other side. But I see these as interesting challenges rather than impediments.

cbegeman commented 1 year ago

@sbrus89 and all, I don't think that config_disable_vel_vmix: true, unless there's a change to the way the implicit drag schemes will be handled?

@dengwirda Thanks for pointing this out. @sbrus89 and I just had a verbal conversation yesterday about implicit drag being supported now for single layer cases and agree with you.

cbegeman commented 1 year ago

Testing:

@sbrus89 I just checked that these namelist settings work with the single-layer dam_break and drying_slope test cases in compass. The pressure gradient type doesn't have to be 'ssh_gradient' to produce reasonable results but I think we should keep 'ssh_gradient' in the yaml file because that's our preference for single_layer cases.

sbrus89 commented 1 year ago

Thanks for the discussion everyone!