Fix the CreateFunction / CreateField split

With Function<T> being the object that handles evaluatable fields and
Field<T> being the non-evaluatable object that can be used for sending
over the network. This commit makes the function naming/usage consisent.

Author: jacobmerson

src/pcms/capi/client.cpp

@@ -1,5 +1,6 @@
 #include "client.h"
 #include "pcms.h"
+#include "pcms/field/field_factory.h"
 #include "pcms/field/function_space/xgc.h"
 #include "pcms/field/data/xgc.h"
 #include "pcms/field/layout/xgc.h"
@@ -23,11 +24,11 @@ namespace detail
 template <typename T>
 struct XGCFieldRegistration
 {
-  XGCFunctionSpace function_space;
+  XGCFieldFactory function_space;
   MPI_Comm plane_comm;
   Rank1View<T, HostMemorySpace> data;
 
-  XGCFieldRegistration(XGCFunctionSpace fs, MPI_Comm comm,
+  XGCFieldRegistration(XGCFieldFactory fs, MPI_Comm comm,
                        Rank1View<T, HostMemorySpace> d)
     : function_space(std::move(fs)), plane_comm(comm), data(d)
   {
@@ -37,10 +38,10 @@ struct XGCFieldRegistration
 struct DummyFieldRegistration
 {};
 
-class EmptyFunctionSpace : public FunctionSpace
+class EmptyFieldFactory : public FieldFactory
 {
 public:
-  explicit EmptyFunctionSpace(std::string layout_name = "")
+  explicit EmptyFieldFactory(std::string layout_name = "")
   {
     auto layout = std::make_shared<EmptyFieldLayout>();
     layout->SetName(std::move(layout_name));
@@ -53,11 +54,6 @@ class EmptyFunctionSpace : public FunctionSpace
     return layout_;
   }
 
-  [[nodiscard]] CoordinateSystem GetCoordinateSystem() const noexcept override
-  {
-    return CoordinateSystem::Cartesian;
-  }
-
 protected:
   [[nodiscard]] FieldVariant CreateFieldImpl(
     Type value_type, FieldMetadata metadata) const override
@@ -80,25 +76,19 @@ class EmptyFunctionSpace : public FunctionSpace
         PCMS_ALWAYS_ASSERT(fd != nullptr);
         if (dynamic_cast<const SimpleFieldData<T>*>(fd.get()) == nullptr) {
           throw pcms_error(
-            "EmptyFunctionSpace::CreateField: requires SimpleFieldData");
+            "EmptyFieldFactory::CreateField: requires SimpleFieldData");
         }
         if (fd->GetDOFHolderDataHost().size() !=
             detail::ExpectedFlatFieldDataSize(*layout_)) {
           throw pcms_error(
-            "EmptyFunctionSpace::CreateField: field data size does not match "
+            "EmptyFieldFactory::CreateField: field data size does not match "
             "layout");
         }
         return WrapField<T>(layout_, std::forward<decltype(fd)>(fd));
       },
       std::move(data));
   }
 
-  [[nodiscard]] PointEvaluatorVariant CreatePointEvaluatorImpl(
-    Type /*value_type*/, const EvaluationRequest& /*request*/) const override
-  {
-    throw pcms_error("EmptyFunctionSpace does not support point evaluation");
-  }
-
 private:
   std::shared_ptr<const EmptyFieldLayout> layout_;
 };
@@ -153,7 +143,7 @@ inline ClientState::HandleVariant RegisterField(
   Application& app, std::string name, const detail::DummyFieldRegistration&,
   bool participates)
 {
-  auto function_space = detail::EmptyFunctionSpace{name};
+  auto function_space = detail::EmptyFieldFactory{name};
   auto field =
     function_space.CreateField<int>(std::move(name), FieldMetadata{});
   std::unique_ptr<FieldSerializer<int>> serializer =
@@ -258,7 +248,7 @@ void pcms_create_xgc_field_adapter_t(
 {
   PCMS_ALWAYS_ASSERT((size > 0) ? (data != nullptr) : true);
   auto function_space =
-    pcms::XGCFunctionSpace(reverse_classification, in_overlap, size);
+    pcms::XGCFieldFactory(reverse_classification, in_overlap, size);
   pcms::Rank1View<T, pcms::HostMemorySpace> data_view(
     reinterpret_cast<T*>(data), size);
   field_adapter.emplace<pcms::detail::XGCFieldRegistration<T>>(

src/pcms/field/eqdsk_field.h

@@ -12,15 +12,15 @@ namespace pcms
 
 struct EQDSKField
 {
-  SplineFunctionSpace space;
+  std::shared_ptr<SplineFunctionSpace> space;
   Field<Real> field;
 };
 
 // TODD: @jacobmerson replace with Function.
 struct EQDSKFieldWithData
 {
   EQDSKData data;
-  SplineFunctionSpace space;
+  std::shared_ptr<SplineFunctionSpace> space;
   Field<Real> field;
 };
 
@@ -43,7 +43,7 @@ inline EQDSKField MakeEQDSKField(const EQDSKData& eqdsk_data)
 {
   auto space = SplineFunctionSpace::FromUniformGrid(
     eqdsk_data.grid, CoordinateSystem::Cartesian);
-  auto field = space.CreateField<Real>();
+  Field<Real> field = space->CreateFunction<Real>();
   field.GetData().SetDOFHolderData(Rank2View<const Real, DeviceMemorySpace>(
     eqdsk_data.PSIZR.data(), eqdsk_data.PSIZR.extent(0), 1));
   return {std::move(space), std::move(field)};

src/pcms/field/field.h

@@ -68,8 +68,10 @@ class Field
   {
   }
 
-  // FunctionSpace constructs Fields (via WrapField) and moves a Field's name,
-  // layout and data out to build a Function (via CreateFunction).
+  // FieldFactory constructs Fields (via WrapField); FunctionSpace constructs
+  // Fields the same way and moves a Field's name, layout and data out to build
+  // a Function (via CreateFunction). Both stamp the field name.
+  friend class FieldFactory;
   friend class FunctionSpace;
 
 private:

src/pcms/field/field_factory.h

@@ -0,0 +1,101 @@
+#ifndef PCMS_FIELD_FACTORY_H
+#define PCMS_FIELD_FACTORY_H
+
+#include "field.h"
+#include "field_data.h"
+#include "field_layout.h"
+#include "field_metadata.h"
+#include "pcms/utility/types.h"
+#include <memory>
+#include <string>
+
+namespace pcms
+{
+
+namespace detail
+{
+
+inline size_t ExpectedFlatFieldDataSize(const FieldLayout& layout)
+{
+  return static_cast<size_t>(layout.GetNumOwnedDofHolder()) *
+         static_cast<size_t>(layout.GetNumComponents());
+}
+
+} // namespace detail
+
+// Compile-time gate: true only for the five supported field value types.
+template <typename T>
+inline constexpr bool is_supported_field_type_v =
+  std::is_same_v<T, int8_t> || std::is_same_v<T, int32_t> ||
+  std::is_same_v<T, int64_t> || std::is_same_v<T, float> ||
+  std::is_same_v<T, double>;
+
+// FieldFactory constructs Field<T> bundles ({layout, data}) with no evaluation
+// capability and no shared-ownership requirement. Comm-only backends (e.g.
+// XGCFieldFactory) derive it directly. FunctionSpace is a separate abstraction
+// (not a FieldFactory) that additionally supports evaluation and produces
+// Functions; a FieldFactory can never be passed where a FunctionSpace is
+// required, which is the compile-time form of "comm-only cannot be evaluated".
+class FieldFactory
+{
+public:
+  virtual std::shared_ptr<const FieldLayout> GetLayout() const noexcept = 0;
+
+  virtual ~FieldFactory() noexcept = default;
+
+  // Create a new named field with freshly allocated data. The name is optional
+  // (empty by default) and identifies the field to consumers such as a coupler.
+  template <typename T>
+  [[nodiscard]] Field<T> CreateField(std::string name = "",
+                                     FieldMetadata metadata = {}) const;
+
+  // Expert API: wrap externally constructed field data into a Field. The
+  // concrete factory validates backend-specific field-data type and storage
+  // size compatibility.
+  template <typename T>
+  [[nodiscard]] Field<T> CreateField(std::string name,
+                                     std::unique_ptr<FieldData<T>> data) const;
+
+protected:
+  template <typename T>
+  static Field<T> WrapField(std::shared_ptr<const FieldLayout> layout,
+                            std::unique_ptr<FieldData<T>> data)
+  {
+    return Field<T>(std::string{}, std::move(layout), std::move(data));
+  }
+
+  virtual FieldVariant CreateFieldImpl(Type value_type,
+                                       FieldMetadata metadata) const = 0;
+
+  virtual FieldVariant CreateFieldImpl(FieldDataVariant data) const = 0;
+};
+
+template <typename T>
+Field<T> FieldFactory::CreateField(std::string name, FieldMetadata metadata) const
+{
+  static_assert(is_supported_field_type_v<T>,
+                "T is not a supported field type");
+  Field<T> field =
+    std::get<Field<T>>(CreateFieldImpl(TypeEnumFromType<T>(), metadata));
+  field.name_ = std::move(name);
+  return field;
+}
+
+template <typename T>
+Field<T> FieldFactory::CreateField(std::string name,
+                                   std::unique_ptr<FieldData<T>> data) const
+{
+  static_assert(is_supported_field_type_v<T>,
+                "T is not a supported field type");
+  if (!data) {
+    throw pcms_error("FieldFactory::CreateField: data must not be null");
+  }
+  Field<T> field =
+    std::get<Field<T>>(CreateFieldImpl(FieldDataVariant{std::move(data)}));
+  field.name_ = std::move(name);
+  return field;
+}
+
+} // namespace pcms
+
+#endif // PCMS_FIELD_FACTORY_H

src/pcms/field/function_space.h

@@ -5,7 +5,7 @@
 #include "evaluation_request.h"
 #include "field.h"
 #include "field_data.h"
-#include "field_evaluator_factory.h"
+#include "field_factory.h"
 #include "field_layout.h"
 #include "field_metadata.h"
 #include "out_of_bounds_policy.h"
@@ -20,37 +20,17 @@
 namespace pcms
 {
 
-namespace detail
-{
-
-inline size_t ExpectedFlatFieldDataSize(const FieldLayout& layout)
-{
-  return static_cast<size_t>(layout.GetNumOwnedDofHolder()) *
-         static_cast<size_t>(layout.GetNumComponents());
-}
-
-} // namespace detail
-
-// Compile-time gate: true only for the five supported field value types.
-template <typename T>
-inline constexpr bool is_supported_field_type_v =
-  std::is_same_v<T, int8_t> || std::is_same_v<T, int32_t> ||
-  std::is_same_v<T, int64_t> || std::is_same_v<T, float> ||
-  std::is_same_v<T, double>;
-
 // FunctionSpace is an abstract interface representing an evaluatable field
-// space: layout, evaluation rules, and coordinate interpretation.
+// space: layout, evaluation rules, and coordinate interpretation. It produces
+// Functions (a Field bound to this space).
 //
 // Concrete implementations (e.g. LagrangeFunctionSpace) provide backends for
-// specific discretizations or mesh types.
-//
-// FunctionSpace is used as the parameter type for operation objects such as
-// Interpolator<T>, so that operations are not coupled to a specific backend.
-//
-// FunctionSpace inherits enable_shared_from_this because CreateFunction stamps
-// a shared_ptr<const FunctionSpace> into each Function it produces. Concrete
-// spaces must therefore be owned by a shared_ptr (the From* factories return
-// one); calling CreateFunction on a stack-allocated space throws bad_weak_ptr.
+// specific discretizations or mesh types, and their From* factories return a
+// shared_ptr. A Function co-owns its space via shared_ptr (obtained through
+// enable_shared_from_this), which is why concrete spaces are always
+// shared-owned; a stack-allocated FunctionSpace is not constructible (the
+// backend constructors are private and reachable only via the shared_ptr
+// From* factories).
 class FunctionSpace : public std::enable_shared_from_this<FunctionSpace>
 {
 public:
@@ -66,28 +46,13 @@ class FunctionSpace : public std::enable_shared_from_this<FunctionSpace>
 
   virtual ~FunctionSpace() noexcept = default;
 
-  // Create a new named field with freshly allocated data for this function
-  // space. The name is optional (empty by default) and identifies the field to
-  // consumers such as a coupler. Compile-time error for unsupported T; runtime
-  // error for T unsupported by the concrete backend.
-  template <typename T>
-  [[nodiscard]] Field<T> CreateField(std::string name = "",
-                                     FieldMetadata metadata = {}) const;
-
-  // Expert API: wrap externally constructed field data into a Field for this
-  // function space. The concrete function space validates backend-specific
-  // field-data type and storage size compatibility.
-  template <typename T>
-  [[nodiscard]] Field<T> CreateField(std::string name,
-                                     std::unique_ptr<FieldData<T>> data) const;
-
-  // Like CreateField, but returns a Function that retains a shared reference to
-  // this space (so it can be evaluated / used to build transfer operators).
-  // Requires this space to be owned by a shared_ptr (see class note).
   template <typename T>
   [[nodiscard]] Function<T> CreateFunction(std::string name = "",
                                            FieldMetadata metadata = {}) const;
 
+  // Expert API: wrap externally constructed field data into a Function for this
+  // space. The concrete space validates backend-specific field-data type and
+  // storage size compatibility.
   template <typename T>
   [[nodiscard]] Function<T> CreateFunction(
     std::string name, std::unique_ptr<FieldData<T>> data) const;
@@ -132,70 +97,30 @@ class FunctionSpace : public std::enable_shared_from_this<FunctionSpace>
     Type value_type, const EvaluationRequest& request) const = 0;
 };
 
-// Wrap a by-value FunctionSpace (typically the result of a From* factory) in a
-// shared_ptr so it can produce Functions and participate in transfers via
-// AddFunction/CreateTransfer. Ordinary by-value use — CreateField, point
-// evaluation, constructing a transfer operator directly — does not need this;
-// only Function (which retains a shared_ptr<const FunctionSpace>) does.
-//
-//   auto space = pcms::Share(LagrangeFunctionSpace::FromMesh(...));
-//   auto fn = space->CreateFunction<Real>();
-template <typename Space>
-[[nodiscard]] std::shared_ptr<Space> Share(Space&& space)
-{
-  static_assert(
-    std::is_base_of_v<FunctionSpace, std::remove_reference_t<Space>>,
-    "Share expects a FunctionSpace-derived type");
-  return std::make_shared<std::remove_reference_t<Space>>(
-    std::forward<Space>(space));
-}
-
 template <typename T>
-Field<T> FunctionSpace::CreateField(std::string name,
-                                    FieldMetadata metadata) const
+Function<T> FunctionSpace::CreateFunction(std::string name,
+                                          FieldMetadata metadata) const
 {
   static_assert(is_supported_field_type_v<T>,
                 "T is not a supported field type");
   Field<T> field =
     std::get<Field<T>>(CreateFieldImpl(TypeEnumFromType<T>(), metadata));
-  field.name_ = std::move(name);
-  return field;
+  return WrapFunction<T>(std::move(name), std::move(field.layout_),
+                         std::move(field.data_), shared_from_this());
 }
 
 template <typename T>
-Field<T> FunctionSpace::CreateField(std::string name,
-                                    std::unique_ptr<FieldData<T>> data) const
+Function<T> FunctionSpace::CreateFunction(
+  std::string name, std::unique_ptr<FieldData<T>> data) const
 {
   static_assert(is_supported_field_type_v<T>,
                 "T is not a supported field type");
   if (!data) {
-    throw pcms_error("FunctionSpace::CreateField: data must not be null");
+    throw pcms_error("FunctionSpace::CreateFunction: data must not be null");
   }
   Field<T> field =
     std::get<Field<T>>(CreateFieldImpl(FieldDataVariant{std::move(data)}));
-  field.name_ = std::move(name);
-  return field;
-}
-
-template <typename T>
-Function<T> FunctionSpace::CreateFunction(std::string name,
-                                          FieldMetadata metadata) const
-{
-  static_assert(is_supported_field_type_v<T>,
-                "T is not a supported field type");
-  Field<T> field = CreateField<T>(std::move(name), metadata);
-  return WrapFunction<T>(std::move(field.name_), std::move(field.layout_),
-                         std::move(field.data_), shared_from_this());
-}
-
-template <typename T>
-Function<T> FunctionSpace::CreateFunction(
-  std::string name, std::unique_ptr<FieldData<T>> data) const
-{
-  static_assert(is_supported_field_type_v<T>,
-                "T is not a supported field type");
-  Field<T> field = CreateField<T>(std::move(name), std::move(data));
-  return WrapFunction<T>(std::move(field.name_), std::move(field.layout_),
+  return WrapFunction<T>(std::move(name), std::move(field.layout_),
                          std::move(field.data_), shared_from_this());
 }