GEOS-ESM / MAPL

MAPL is a foundation layer of the GEOS architecture, whose original purpose is to supplement the Earth System Modeling Framework (ESMF)
https://geos-esm.github.io/MAPL/
Apache License 2.0
26 stars 17 forks source link

Refactor VariableSpec and StateItemSpec #2980

Open tclune opened 2 months ago

tclune commented 2 months ago

For indirect reasons, it would now be preferable if subclasses of StateItemSpec have trivial constructors with an initialize() method doing more of the heavy lifting.

This could be done in two ways: (Consider FieldSpec is the poster child for StateItemSpec subclasses)

  1. field_spec = FieldSpec() ... call field_spec%initialize(variable_spec, _RC)
  2. field_spec = FieldSpec(variable_spec) ... call field_spec(_RC)

Originally I was thinking (1), but (2) has several advantages. In particular, it means that VariableSpec does not appear in the interfaces of StateItemSpec, and this may allow make_ItemSpec() to continue to live right where it is and require less total refactoring.

For now the call to initialize() can remain inside of make_ItemSpec(). Moving that to a new home is something I can do when I'm ready to exercise this functionality.

I will need this soon, so please be honest about level of effort you can apply.

darianboggs commented 2 months ago

There may be a dependency chain:

  1. mapl3g_VariableSpec depends on mapl3g_StateItemSpec and mapl3g_FieldSpec which in turn depends on mapl3g_StateIemSpec, currently.
  2. We want VariableSpec to pass itself into FieldSpec in the constructor called from make_itemSpec
  3. Therefore, the constructor for FieldSpec needs a VariableSpec argument.
  4. So, mapl3g_FieldSpec needs to depend on mapl3g_VariableSpec.

Is that correct? I can work around it, but I want to confirm:

  1. We don't want mapl3g_StateItemSpec to depend on mapl3g_VariableSpec.
  2. We want to defer initialization of FieldSpec objects after they are constructed.
darianboggs commented 2 months ago

I think this:

  1. field_spec = FieldSpec(variable_spec) ... call field_spec(_RC)

should be this:

field_spec = FieldSpec(variable_spec) ... call field_spec%initialize(_RC)
tclune commented 2 months ago

I think this:

  1. field_spec = FieldSpec(variable_spec) ... call field_spec(_RC)

should be this:

field_spec = FieldSpec(variable_spec) ... call field_spec%initialize(_RC)

Yes. Sorry.