Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

initial Interfacer cleanup #644

Merged
merged 1 commit into from
Mar 6, 2024
Merged

initial Interfacer cleanup #644

merged 1 commit into from
Mar 6, 2024

Conversation

juliasloan25
Copy link
Member

@juliasloan25 juliasloan25 commented Feb 27, 2024

Purpose

First steps of Interfacer cleanup. Some of it (e.g. restructuring) will be done in separate PRs so this one doesn't get too big.

closes #339

To-do

Content

get_field

  • add get_field(surfacestub, :air_density)
  • remove get_field(atmossim, :cv_m) and get_field(atmossim, :gas_constant_air)
  • get_field, update_field: implement default for each variable we expect, output warning with maxlog = 1 for each
  • alphabetize get_field and update_field functions for readability

surfacestub

  • add stub_init function (just wrap SurfaceStub constructor)

renaming variables

  • rename co2_gm -> co2 (explain in docs global mean)
  • rename albedo -> surface_albedo (atmos update_field, surfaces get_field)
  • rename clean_sst, clean_sic -> scale_sst, scale_sic
  • rename get_model_state_vector -> get_model_prog_state

restructuring

  • differentiate_turbulent_fluxes: dispatch on eisenman sea ice type, move to eisenman file (specific to that model)

docs

  • note that update_field(::atmossim, turbulent_fluxes) required for partitionedfluxes option
  • note: update_sim! can be optionally overwritten, but not required
  • update_turb_fluxes_point required for surface model if using partitioned sf
  • get_model_state_vector required for checkpointing
  • note in docs: surface model precip only needs update, get not required
  • explain that CO2 from atmos is expected to be global mean
  • add units to docs

QA

  • test update_field! warnings
  • check docs rendering

To Do in future PRs

  • component model files: rename each to <model>.jl, combine bucket_init and bucket_utils -> bucket.jl; separate required vs model-specific (internal) functions and objects in component model files - see restructure component models #647
  • separate surfacestub from Interfacer/FieldExchanger move SurfaceStub to its own file #669
    • move surfacestub to separate file outside of Interfacer.jl, and include from Interfacer.jl so it's still in that module
    • same for FieldExchanger.jl

Remaining questions

  • to run with partitioned fluxes, does the atmos model need to extend update_turbulent_fluxes_point? or just surface models as it is currently?
    • No, update_turbulent_fluxes_point is called by surface models only. Once the surfaces are updated and fluxes per each fractional surface calculated, the coupler combines the fluxes so that we have one flux value per atmos grid point, and we update atmos in the same way as for the CombinedStateFluxes. I will work on the flux calculation interface in a few weeks, with the aim of making this function more transparent and performant. :)
  • Are fields in component models (e.g. those returned by get_field) expected to be 2d or just individual floats? Does it vary e.g. for surface albedo vs co2 global mean? (might want to clarify in docs)
    • We should be able to handle both, and (for now) co2 stays constant in space and variable in time.

  • I have read and checked the items on the review checklist.

Copy link
Collaborator

@LenkaNovak LenkaNovak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great, Julia, thank you! I left a few comments. If you need help with any of TODOs, let me know once everything else is good to go and I can add the remaining info.

docs/src/interfacer.md Outdated Show resolved Hide resolved
docs/src/interfacer.md Outdated Show resolved Hide resolved
docs/src/interfacer.md Outdated Show resolved Hide resolved
docs/src/interfacer.md Outdated Show resolved Hide resolved
docs/src/interfacer.md Outdated Show resolved Hide resolved
docs/src/interfacer.md Outdated Show resolved Hide resolved
src/FluxCalculator.jl Outdated Show resolved Hide resolved
src/Interfacer.jl Show resolved Hide resolved
src/Interfacer.jl Outdated Show resolved Hide resolved
src/Interfacer.jl Outdated Show resolved Hide resolved
@juliasloan25 juliasloan25 force-pushed the js/interfacer branch 4 times, most recently from 7862425 to 08e6cd2 Compare March 5, 2024 22:01
@juliasloan25 juliasloan25 merged commit ef2b3ce into main Mar 6, 2024
6 of 7 checks passed
@juliasloan25 juliasloan25 mentioned this pull request Mar 8, 2024
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

clean up and document coupler fields
2 participants