Rework of PKs to split out parseParameterList, make RequireEvaluator(…

…) call EnsureEvaluators() (#270)

* adds a new PK method, parseParameterList(), pulling that out of constructors.
* with Amanzi amanzi/amanzi#860 we now know that, after calling RequireEvaluator(), the sub-DAG rooted at that eval is valid, and can be used to call IsDifferentiableWRT(), meaning that PKs can now detect this and require derivatives without asking the user.  closes #167

src/executables/coordinator.cc

@@ -284,9 +284,11 @@ Coordinator::setup()
   S_->require_time(Amanzi::Tags::CURRENT);
   S_->require_time(Amanzi::Tags::NEXT);
 
-  // order matters here -- PKs set the leaves, then observations can use those
-  // if provided, and setup finally deals with all secondaries and allocates memory
+  // order matters here -- PK::Setup() set the leaves, then observations can
+  // use those if provided, and State::Setup finally deals with all secondaries
+  // and allocates memory
   pk_->set_tags(Amanzi::Tags::CURRENT, Amanzi::Tags::NEXT);
+  pk_->parseParameterList();
   pk_->Setup();
   for (auto& obs : observations_) obs->Setup(S_.ptr());
   S_->Setup();

src/executables/elm_ats_api/elm_ats_driver.cc

@@ -212,22 +212,17 @@ ELM_ATSDriver::setup()
   requireAtNext(wc_key_, Amanzi::Tags::NEXT, *S_)
     .SetMesh(mesh_subsurf_)->AddComponent("cell", AmanziMesh::CELL, 1);
 
-
-
   requireAtNext(evap_key_, Amanzi::Tags::NEXT, *S_)
    .SetMesh(mesh_surf_)->AddComponent("cell", AmanziMesh::CELL, 1);
   requireAtNext(trans_key_, Amanzi::Tags::NEXT, *S_)
    .SetMesh(mesh_subsurf_)->AddComponent("cell", AmanziMesh::CELL, 1);
 
-
-  Coordinator::setup();
-  // NOTE: These must be called after Coordinator::setup() until PK_Phys_Default modifies the state eval
-  // list in constructor -- otherwise this primary variable is not available
-  // yet. See amanzi/ats#167 --ETC
   requireAtNext(infilt_key_, Amanzi::Tags::NEXT, *S_)
     .SetMesh(mesh_surf_)->AddComponent("cell", AmanziMesh::CELL, 1);
   requireAtNext(pres_key_, Amanzi::Tags::NEXT, *S_, "flow")
     .SetMesh(mesh_subsurf_)->AddComponent("cell", AmanziMesh::CELL, 1);
+
+  Coordinator::setup();
 }
 
 

src/executables/test/executable_coupled_water.cc

@@ -77,6 +77,7 @@ struct CoupledWaterProblem {
     Amanzi::PKFactory pk_factory;
     auto pk_as_pk = pk_factory.CreatePK("coupled water", pk_tree_list, plist, S, soln);
     pk = Teuchos::rcp_dynamic_cast<MPCCoupledWater>(pk_as_pk);
+    pk->parseParameterList();
     AMANZI_ASSERT(pk.get());
 
     // setup stage

src/pks/deform/volumetric_deformation.cc

@@ -30,7 +30,14 @@ VolumetricDeformation::VolumetricDeformation(Teuchos::ParameterList& pk_tree,
     PK_Physical_Default(pk_tree, glist, S, solution),
     surf_mesh_(Teuchos::null),
     deformed_this_step_(false)
+{}
+
+
+void
+VolumetricDeformation::parseParameterList()
 {
+  PK_Physical_Default::parseParameterList();
+
   dt_max_ = plist_->get<double>("max time step [s]", std::numeric_limits<double>::max());
 
   // The deformation mode describes how to calculate new cell volume from a

src/pks/deform/volumetric_deformation.hh

@@ -129,7 +129,8 @@ class VolumetricDeformation : public PK_Physical_Default {
                         const Teuchos::RCP<State>& S,
                         const Teuchos::RCP<TreeVector>& solution);
 
-  // ConstantTemperature is a PK
+  void parseParameterList() override;
+
   // -- Setup data
   virtual void Setup() override;
 

src/pks/energy/energy_base.hh

@@ -74,13 +74,11 @@ Solves an advection-diffusion equation for energy:
     * `"source key`" ``[string]`` **DOMAIN-total_energy_source** Typically
       not set, as the default is good. ``[MJ s^-1]``
 
-    * `"source term is differentiable`" ``[bool]`` **true** Can the source term
-      be differentiated with respect to the primary variable?
-
-    * `"source term finite difference`" ``[bool]`` **false** If the source term
-      is not diffferentiable, we can do a finite difference approximation of
-      this derivative anyway.  This is useful for difficult-to-differentiate
-      terms like a surface energy balance, which includes many terms.
+    * `"source term finite difference`" ``[bool]`` **false** Option to do a
+      finite difference approximation of the source term's derivative with
+      respect to the primary variable.  This is useful for
+      difficult-to-differentiate terms like a surface energy balance, which
+      includes many terms.
 
     END
 
@@ -182,6 +180,9 @@ class EnergyBase : public PK_PhysicalBDF_Default {
   // Virtual destructor
   virtual ~EnergyBase() {}
 
+  // call to allow a PK to modify its own list or lists of its children.
+  virtual void parseParameterList() override;
+
   // EnergyBase is a PK
   // -- Setup data
   virtual void Setup() override;

src/pks/energy/energy_base_physics.cc

@@ -161,32 +161,32 @@ void
 EnergyBase::AddSourcesToPrecon_(double h)
 {
   // external sources of energy (temperature dependent source)
-  if (is_source_term_ && is_source_term_differentiable_ &&
-      S_->GetEvaluator(source_key_, tag_next_).IsDifferentiableWRT(*S_, key_, tag_next_)) {
-    Teuchos::RCP<CompositeVector> dsource_dT;
-
-    if (is_source_term_finite_differentiable_) {
-      // evaluate the derivative through finite differences
-      double eps = 1.e-8;
-      S_->GetW<CompositeVector>(key_, tag_next_, name_).Shift(eps);
-      ChangedSolution();
-      S_->GetEvaluator(source_key_, tag_next_).Update(*S_, name_);
-      auto dsource_dT_nc =
-        Teuchos::rcp(new CompositeVector(S_->Get<CompositeVector>(source_key_, tag_next_)));
-
-      S_->GetW<CompositeVector>(key_, tag_next_, name_).Shift(-eps);
-      ChangedSolution();
-      S_->GetEvaluator(source_key_, tag_next_).Update(*S_, name_);
-
-      dsource_dT_nc->Update(-1 / eps, S_->Get<CompositeVector>(source_key_, tag_next_), 1 / eps);
-      dsource_dT = dsource_dT_nc;
-
-    } else {
-      // evaluate the derivative through the dag
-      S_->GetEvaluator(source_key_, tag_next_).UpdateDerivative(*S_, name_, key_, tag_next_);
-      dsource_dT = S_->GetDerivativePtrW<CompositeVector>(
-        source_key_, tag_next_, key_, tag_next_, source_key_);
-    }
+  Teuchos::RCP<CompositeVector> dsource_dT(Teuchos::null);
+
+  if (is_source_term_finite_differentiable_) {
+    // evaluate the derivative through finite differences
+    double eps = 1.e-8;
+    S_->GetW<CompositeVector>(key_, tag_next_, name_).Shift(eps);
+    ChangedSolution();
+    S_->GetEvaluator(source_key_, tag_next_).Update(*S_, name_);
+    auto dsource_dT_nc =
+      Teuchos::rcp(new CompositeVector(S_->Get<CompositeVector>(source_key_, tag_next_)));
+
+    S_->GetW<CompositeVector>(key_, tag_next_, name_).Shift(-eps);
+    ChangedSolution();
+    S_->GetEvaluator(source_key_, tag_next_).Update(*S_, name_);
+
+    dsource_dT_nc->Update(-1 / eps, S_->Get<CompositeVector>(source_key_, tag_next_), 1 / eps);
+    dsource_dT = dsource_dT_nc;
+
+  } else if (is_source_term_differentiable_) {
+    // evaluate the derivative through the dag
+    S_->GetEvaluator(source_key_, tag_next_).UpdateDerivative(*S_, name_, key_, tag_next_);
+    dsource_dT = S_->GetDerivativePtrW<CompositeVector>(
+      source_key_, tag_next_, key_, tag_next_, source_key_);
+  }
+
+  if (dsource_dT != Teuchos::null) {
     db_->WriteVector("  dQ_ext/dT", dsource_dT.ptr(), false);
     preconditioner_acc_->AddAccumulationTerm(*dsource_dT, -1.0, "cell", true);
   }

src/pks/energy/energy_base_pk.cc

@@ -48,8 +48,37 @@ EnergyBase::EnergyBase(Teuchos::ParameterList& FElist,
     coupled_to_surface_via_flux_(false),
     decoupled_from_subsurface_(false),
     niter_(0),
-    flux_exists_(true)
+    flux_exists_(true),
+    is_source_term_(false),
+    is_source_term_differentiable_(false),
+    is_source_term_finite_differentiable_(false)
+{}
+
+
+
+// call to allow a PK to modify its own list or lists of its children.
+void
+EnergyBase::parseParameterList()
 {
+  if (!plist_->isParameter("absolute error tolerance")) {
+    plist_->set("absolute error tolerance", 76.e-6);
+    // energy of 1 degree C of water per mass_atol, in MJ/mol water
+  }
+
+  // set some defaults for inherited PKs
+  if (!plist_->isParameter("conserved quantity key suffix"))
+    plist_->set<std::string>("conserved quantity key suffix", "energy");
+
+  // need to deprecate the use of enthalpy list in the PK! --ETC
+  enthalpy_key_ = Keys::readKey(*plist_, domain_, "enthalpy", "enthalpy");
+  if (plist_->isSublist("enthalpy evaluator")) {
+    Teuchos::ParameterList& enth_list = S_->GetEvaluatorList(enthalpy_key_);
+    enth_list.setParameters(plist_->sublist("enthalpy evaluator"));
+    enth_list.set<std::string>("evaluator type", "enthalpy");
+  }
+
+  PK_PhysicalBDF_Default::parseParameterList();
+
   // set a default error tolerance
   if (domain_.find("surface") != std::string::npos) {
     mass_atol_ = plist_->get<double>("mass absolute error tolerance", .01 * 55000.);
@@ -60,23 +89,16 @@ EnergyBase::EnergyBase(Teuchos::ParameterList& FElist,
     // porosity * particle density soil * heat capacity soil * 1 degree
     // or, dry bulk density soil * heat capacity soil * 1 degree, in MJ
   }
-  if (!plist_->isParameter("absolute error tolerance")) {
-    plist_->set("absolute error tolerance", 76.e-6);
-    // energy of 1 degree C of water per mass_atol, in MJ/mol water
-  }
 
   // source terms
   is_source_term_ = plist_->get<bool>("source term", false);
   if (is_source_term_ && source_key_.empty()) {
     source_key_ = Keys::readKey(*plist_, domain_, "source", "total_energy_source");
+    is_source_term_finite_differentiable_ = plist_->get<bool>("source term finite difference", false);
   }
-  is_source_term_differentiable_ = plist_->get<bool>("source term is differentiable", true);
-  is_source_term_finite_differentiable_ = plist_->get<bool>("source term finite difference", false);
 
   // get keys
-  conserved_key_ = Keys::readKey(*plist_, domain_, "conserved quantity", "energy");
   wc_key_ = Keys::readKey(*plist_, domain_, "water content", "water_content");
-  enthalpy_key_ = Keys::readKey(*plist_, domain_, "enthalpy", "enthalpy");
   flux_key_ = Keys::readKey(*plist_, domain_, "water flux", "water_flux");
   energy_flux_key_ =
     Keys::readKey(*plist_, domain_, "diffusive energy flux", "diffusive_energy_flux");
@@ -144,18 +166,12 @@ EnergyBase::SetupEnergy_()
       .SetMesh(mesh_)
       ->AddComponent("cell", AmanziMesh::Entity_kind::CELL, 1);
 
-    if (is_source_term_differentiable_ && (!is_source_term_finite_differentiable_)) {
-      // NOTE, the following line is commented out because of the bug, amanzi/ats#167
-      //&& S_->GetEvaluator(source_key_, tag_next_).IsDifferentiableWRT(*S_, key_, tag_next_)) {
+    if (!is_source_term_finite_differentiable_
+        && S_->GetEvaluator(source_key_, tag_next_).IsDifferentiableWRT(*S_, key_, tag_next_)) {
+      is_source_term_differentiable_ = true;
       // require derivative of source
       S_->RequireDerivative<CompositeVector, CompositeVectorSpace>(
-          source_key_, tag_next_, key_, tag_next_)
-        .SetMesh(mesh_)
-        ->AddComponent("cell", AmanziMesh::Entity_kind::CELL, 1);
-      // NOTE, remove SetMesh/AddComponent lines after fixing amanzi/ats#167.
-      // The mesh should get set by the evaluator, but when
-      // the evaluator isn't actually differentiable, it
-      // doesn't get done.
+        source_key_, tag_next_, key_, tag_next_);
     }
   }
 
@@ -286,11 +302,6 @@ EnergyBase::SetupEnergy_()
   // -- set up the evaluator for enthalpy, whether or not we advect the thing,
   //    we may need it for exchange fluxes with surface/subsurface coupling,
   //    etc.
-  if (plist_->isSublist("enthalpy evaluator")) {
-    Teuchos::ParameterList& enth_list = S_->GetEvaluatorList(enthalpy_key_);
-    enth_list.setParameters(plist_->sublist("enthalpy evaluator"));
-    enth_list.set<std::string>("evaluator type", "enthalpy");
-  }
   is_advection_term_ = plist_->get<bool>("include thermal advection", true);
   if (is_advection_term_) {
     // -- create the forward operator for the advection term

src/pks/flow/constitutive_relations/sources/distributed_tiles_evaluator.hh

@@ -56,6 +56,11 @@ class DistributedTilesRateEvaluator : public EvaluatorSecondary {
 
   virtual void EnsureCompatibility(State& S) override;
 
+  // derivatives aren't implemented here
+  bool IsDifferentiableWRT(const State& S, const Key& wrt_key, const Tag& wrt_tag) const override {
+    return false;
+  }
+
  protected:
   // Required methods from EvaluatorSecondary
   virtual void Update_(State& S) override;

src/pks/flow/overland_pressure.hh

@@ -41,8 +41,6 @@ Solves the diffusion wave equation for overland flow with pressure as a primary
     * `"source key`" ``[string]`` **DOMAIN-water_source** Typically
       not set, as the default is good. ``[m s^-1]`` or ``[mol s^-1]``
     * `"water source in meters`" ``[bool]`` **true** Is the source term in ``[m s^-1]``?
-    * `"source term is differentiable`" ``[bool]`` **true** Can the source term
-      be differentiated with respect to the primary variable?
 
     END
 
@@ -157,6 +155,9 @@ class OverlandPressureFlow : public PK_PhysicalBDF_Default {
   // Virtual destructor
   virtual ~OverlandPressureFlow() {}
 
+  // parse input list
+  virtual void parseParameterList() override;
+
   // main methods
   // -- Initialize owned (dependent) variables.
   virtual void Setup() override;
@@ -265,7 +266,7 @@ class OverlandPressureFlow : public PK_PhysicalBDF_Default {
   Operators::UpwindMethod upwind_method_;
 
   bool is_source_term_;
-  bool source_term_is_differentiable_;
+  bool is_source_term_differentiable_;
   bool source_only_if_unfrozen_;
 
   bool modify_predictor_with_consistent_faces_;

src/pks/flow/overland_pressure_physics.cc

@@ -124,8 +124,7 @@ void
 OverlandPressureFlow::AddSourcesToPrecon_(double h)
 {
   // -- update the source term derivatives
-  if (is_source_term_ && source_term_is_differentiable_ &&
-      S_->GetEvaluator(source_key_, tag_next_).IsDifferentiableWRT(*S_, key_, tag_next_)) {
+  if (is_source_term_differentiable_) {
     S_->GetEvaluator(source_key_, tag_next_).UpdateDerivative(*S_, name_, key_, tag_next_);
     preconditioner_acc_->AddAccumulationTerm(
       S_->GetDerivative<CompositeVector>(source_key_, tag_next_, key_, tag_next_),

src/pks/flow/overland_pressure_pk.cc

@@ -41,6 +41,7 @@ OverlandPressureFlow::OverlandPressureFlow(Teuchos::ParameterList& pk_tree,
     PK_PhysicalBDF_Default(pk_tree, plist, S, solution),
     standalone_mode_(false),
     is_source_term_(false),
+    is_source_term_differentiable_(false),
     coupled_to_subsurface_via_head_(false),
     coupled_to_subsurface_via_flux_(false),
     perm_update_required_(true),
@@ -52,13 +53,24 @@ OverlandPressureFlow::OverlandPressureFlow(Teuchos::ParameterList& pk_tree,
     jacobian_lag_(0),
     iter_(0),
     iter_counter_time_(0.)
+{}
+
+
+void
+OverlandPressureFlow::parseParameterList()
 {
   // set a default absolute tolerance
   if (!plist_->isParameter("absolute error tolerance"))
     plist_->set("absolute error tolerance", 0.01 * 55000.0); // h * nl
 
+  // set some defaults for inherited PKs
+  if (!plist_->isParameter("conserved quantity key suffix"))
+    plist_->set<std::string>("conserved quantity key suffix", "water_content");
+
+  PK_PhysicalBDF_Default::parseParameterList();
+
   // get keys
-  conserved_key_ = Keys::readKey(*plist_, domain_, "conserved quantity", "water_content");
+  potential_key_ = Keys::readKey(*plist_, domain_, "potential", "pres_elev");
   potential_key_ = Keys::readKey(*plist_, domain_, "potential", "pres_elev");
   flux_key_ = Keys::readKey(*plist_, domain_, "water flux", "water_flux");
   flux_dir_key_ = Keys::readKey(*plist_, domain_, "water flux direction", "water_flux_direction");
@@ -78,22 +90,22 @@ OverlandPressureFlow::OverlandPressureFlow(Teuchos::ParameterList& pk_tree,
 
   // alter lists for evaluators
   // -- add _bar evaluators
-  Teuchos::ParameterList& pd_bar_list = S->GetEvaluatorList(pd_bar_key_);
-  pd_bar_list.setParameters(S->GetEvaluatorList(pd_key_));
+  Teuchos::ParameterList& pd_bar_list = S_->GetEvaluatorList(pd_bar_key_);
+  pd_bar_list.setParameters(S_->GetEvaluatorList(pd_key_));
   pd_bar_list.set("allow negative ponded depth", true);
 
-  Teuchos::ParameterList& wc_bar_list = S->GetEvaluatorList(wc_bar_key_);
-  wc_bar_list.setParameters(S->GetEvaluatorList(conserved_key_));
+  Teuchos::ParameterList& wc_bar_list = S_->GetEvaluatorList(wc_bar_key_);
+  wc_bar_list.setParameters(S_->GetEvaluatorList(conserved_key_));
   wc_bar_list.set("allow negative water content", true);
 
   // -- elevation evaluator
-  standalone_mode_ = S->GetMesh() == S->GetMesh(domain_);
-  if (!standalone_mode_ && !S->FEList().isSublist(elev_key_)) {
-    S->GetEvaluatorList(elev_key_).set("evaluator type", "meshed elevation");
+  standalone_mode_ = S_->GetMesh() == S_->GetMesh(domain_);
+  if (!standalone_mode_ && !S_->FEList().isSublist(elev_key_)) {
+    S_->GetEvaluatorList(elev_key_).set("evaluator type", "meshed elevation");
   }
 
   // -- potential evaluator
-  auto& potential_list = S->GetEvaluatorList(potential_key_);
+  auto& potential_list = S_->GetEvaluatorList(potential_key_);
   potential_list.set("evaluator type", "additive evaluator");
   potential_list.set<Teuchos::Array<std::string>>("dependencies",
                                                   std::vector<std::string>{ pd_key_, elev_key_ });
@@ -368,9 +380,8 @@ OverlandPressureFlow::SetupPhysicalEvaluators_()
       .SetMesh(mesh_)
       ->AddComponent("cell", AmanziMesh::Entity_kind::CELL, 1);
 
-
-    source_term_is_differentiable_ = plist_->get<bool>("source term is differentiable", true);
-    if (source_term_is_differentiable_) {
+    if (S_->GetEvaluator(source_key_, tag_next_).IsDifferentiableWRT(*S_, key_, tag_next_)) {
+      is_source_term_differentiable_ = true;
       // require derivative of source
       S_->RequireDerivative<CompositeVector, CompositeVectorSpace>(
         source_key_, tag_next_, key_, tag_next_);

src/pks/flow/permafrost_pk.cc

@@ -74,9 +74,7 @@ Permafrost::SetupPhysicalEvaluators_()
 
   //    and at the current time, where it is a copy evaluator
   requireAtCurrent(sat_key_, tag_current_, *S_, name_, true);
-  // S_->RequireEvaluator(sat_key_, tag_current_);
   requireAtCurrent(sat_ice_key_, tag_current_, *S_, name_, true);
-  // S_->RequireEvaluator(sat_ice_key_, tag_current_);
 
   // -- the rel perm evaluator, also with the same underlying WRM.
   S_->GetEvaluatorList(coef_key_).set<double>("permeability rescaling", perm_scale_);