Repository: NREL/OpenStudio (language: python)

Issue:
Add Utility for Generating Alfalfa Metadata to OpenStudio
 
<!--- DESCRIBE PURPOSE OF THIS PULL REQUEST -->
Alfalfa is a platform which allows users to interact with OpenStudio/EnergyPlus models in real time. In order to accomplish this users must specify what components of the model to connect with Alfalfa as input and output points. This PR moves the mechanisms for creating "Alfalfa Points" into the OpenStudio runner so that they may be accessed during the measure workflow.

### Pull Request Author

<!--- Add to this list or remove from it as applicable.  This is a simple templated set of guidelines. -->

 - [ ] Model API Changes / Additions
 - [ ] Any new or modified fields have been implemented in the EnergyPlus ForwardTranslator (and ReverseTranslator as appropriate)
 - [ ] Model API methods are tested (in `src/model/test`)
 - [ ] EnergyPlus ForwardTranslator Tests (in `src/energyplus/Test`)
 - [ ] If a new object or method, added a test in NREL/OpenStudio-resources: Add Link
 - [ ] If needed, added VersionTranslation rules for the objects (`src/osversion/VersionTranslator.cpp`)
 - [ ] Verified that C# bindings built fine on Windows, partial classes used as needed, etc.
 - [ ] All new and existing tests passes
 - [ ] If methods have been deprecated, update rest of code to use the new methods

**Labels:**

 - [ ] If change to an IDD file, add the label `IDDChange`
 - [ ] If breaking existing API, add the label `APIChange`
 - [ ] If deemed ready, add label `Pull Request - Ready for CI` so that CI builds your PR

### Review Checklist

This will not be exhaustively relevant to every PR.
 - [ ] Perform a Code Review on GitHub
 - [ ] Code Style, strip trailing whitespace, etc.
 - [ ] All related changes have been implemented: model changes, model tests, FT changes, FT tests, VersionTranslation, OS App
 - [ ] Labeling is ok
 - [ ] If defect, verify by running develop branch and reproducing defect, then running PR and reproducing fix
 - [ ] If feature, test running new feature, try creative ways to break it
 - [ ] CI status: all green or justified



Discussion / hints:
@kbenne what do you think about the language of `add` vs something like `export`.

Right now all the `AlfalfaJSON` methods are phrased as `add` like `addGlobalVariable`, `addMeter`, etc. I wonder if it might make more sense to replace `add` with `export` since what you're doing is exporting a `GlobalVariable`, `Meter`, `OutputVariable`, etc. to Alfalfa so it may be consumed there. 
@kbenne this is pretty much there.
My TODO:
- [x] Finish json serialization saving and connection with existing OS workflow.
- [ ] Audit if there are additional Model Objects of IDF objects that we should support automatic point creation for
- [ ] Add methods for exposing lists of variables from one object, as seen in `EnergyManagementSystem:GlobalVariable`
@TShapinsky as we discussed, it would be a good idea to test a workflow that excerses this new feature. You can see an example of workflow tests [here](https://github.com/NREL/OpenStudio/blob/ba1b94b6542c1f1d4cdcdb775886852b9a82bc4f/src/cli/CMakeLists.txt#L212). Note they are not Google Tests, but instead these are implemented purely in the CTest wrapper. 
I created a new [issue](https://github.com/NREL/OpenStudio/issues/5250) to create documentation for this new API.
@kbenne I'm happy where everything is, with the exception of the behaviour for dealing with duplicate ids. One possible approach is to modify duplicate ids on export so they become unique. This would work, but it could be confusing and it is also non-deterministic. If someone was expecting a certain set of IDs from a measure they might not get them if a previous measure adds points of the same id. 

Another approach is to throw an error on export which could be annoying. It also runs the possibility of weird behaviour where based on naming of model objects certain models would error while others didn't. Are names unique across an IDF? or are they just unique to an idd type?

A possible third approach would be to refactor the id out of the point and have it be set in the AlfalfaJSON. This is probably the most correct as it is a key used to refer to the point and thus not something that the point needs to know about. This would allow for runtime checking of point Id against the existing points.

I think the best option may be a combination of the first and third. Refactoring the point id to be stored in the AlfalfaJSON object and dynamically renaming point ids to prevent overlap. A warning could also be printed to alert of this happening. But that would only be shown locally, so it wouldn't include weird behaviour that happens when the measures interact with the baked in alfalfa measures.

Maybe after this we need to reevaluate if we want to be automatically baking points into alfalfa or if we should just provide the measures we create for generating generic points as a resource for people putting together workflows. That would allow for the maximum amount of reproducibility between alfalfa and local development.
@jmarrec I would grateful to have you weigh in on this. The challenge is that I'm not sure that you have enough context or any available time. If you don't have time, that's understandable, just say so. My intention is to merge this in time for the next release unless there are any serious red flags that emerge.

cc @DavidGoldwasser @TShapinsky 
@wenyikuang can you investigate the CI failures?
Looks the windows-incremental is lost connection from the jenkins, let me look
If i remember correctly, the osx build fails were from tests failing for BCL measures.
Resolve the issue of connection between jenkins and windows-incre-on-aws

https://ci.openstudio.net/blue/organizations/jenkins/openstudio-incremental-windows/detail/PR-5255/7/pipeline

(From another PR but using the same node)

Or are you mentioning these one?


```
The following tests FAILED:

	665 - BCLFixture.RemoteBCL_BCLSearchResult (Failed)

	1877 - ModelFixture.PythonPluginInstance_NotPYFile (Failed)

	2304 - ModelFixture.ScheduleFile (Failed)

	3771 - OpenStudioCLI.Check_Alfalfa (Failed)
```


@wenyikuang cppcheck is broken, you should fix that: https://github.com/NREL/OpenStudio/actions/runs/10853524579/job/30432985755#step:3:16

Fix patch:
diff --git a/src/utilities/alfalfa/AlfalfaActuator.cpp b/src/utilities/alfalfa/AlfalfaActuator.cpp
index 2e1b85b5720..dd46541c6cf 100644
--- a/src/utilities/alfalfa/AlfalfaActuator.cpp
+++ b/src/utilities/alfalfa/AlfalfaActuator.cpp
@@ -1,6 +1,6 @@
 #include "AlfalfaActuator.hpp"
-#include "utilities/idd/IddObject.hpp"
-#include "utilities/idd/IddEnums.hpp"
+#include "../idd/IddObject.hpp"
+#include "../idd/IddEnums.hpp"
 
 #include <utilities/idd/OS_EnergyManagementSystem_Actuator_FieldEnums.hxx>
 #include <utilities/idd/EnergyManagementSystem_Actuator_FieldEnums.hxx>
@@ -12,11 +12,11 @@ namespace openstudio {
 namespace alfalfa {
   AlfalfaActuator::AlfalfaActuator(const std::string& component_name, const std::string& component_type, const std::string& control_type)
     : AlfalfaComponent("Actuator", Capability::Input | Capability::Output) {
-    if (component_name.size() == 0) {
+    if (component_name.empty()) {
       throw std::runtime_error("Error creating AlfalfaActuator: component_name must be non-empty");
-    } else if (component_type.size() == 0) {
+    } else if (component_type.empty()) {
       throw std::runtime_error("Error creating AlfalfaActuator: component_type must be non-empty");
-    } else if (control_type.size() == 0) {
+    } else if (control_type.empty()) {
       throw std::runtime_error("Error creating AlfalfaActuator: control_type must be non-empty");
     }
     parameters["component_name"] = component_name;
@@ -27,35 +27,35 @@ namespace alfalfa {
   AlfalfaActuator::AlfalfaActuator(const IdfObject& actuator) : AlfalfaComponent("Actuator", Capability::Input | Capability::Output) {
     IddObjectType idd_type = actuator.iddObject().type();
 
-    boost::optional<std::string> component_name;
-    boost::optional<std::string> component_type;
-    boost::optional<std::string> control_type;
+    std::string component_name;
+    std::string component_type;
+    std::string control_type;
 
     if (idd_type == IddObjectType::OS_EnergyManagementSystem_Actuator) {
-      component_name = actuator.getString(OS_EnergyManagementSystem_ActuatorFields::ActuatedComponentName);
-      component_type = actuator.getString(OS_EnergyManagementSystem_ActuatorFields::ActuatedComponentType);
-      control_type = actuator.getString(OS_EnergyManagementSystem_ActuatorFields::ActuatedComponentControlType);
+      component_name = actuator.getString(OS_EnergyManagementSystem_ActuatorFields::ActuatedComponentName).value_or("");
+      component_type = actuator.getString(OS_EnergyManagementSystem_ActuatorFields::ActuatedComponentType).value_or("");
+      control_type = actuator.getString(OS_EnergyManagementSystem_ActuatorFields::ActuatedComponentControlType).value_or("");
     } else if (idd_type == IddObjectType::EnergyManagementSystem_Actuator) {
-      component_name = actuator.getString(EnergyManagementSystem_ActuatorFields::ActuatedComponentUniqueName);
-      component_type = actuator.getString(EnergyManagementSystem_ActuatorFields::ActuatedComponentType);
-      control_type = actuator.getString(EnergyManagementSystem_ActuatorFields::ActuatedComponentControlType);
+      component_name = actuator.getString(EnergyManagementSystem_ActuatorFields::ActuatedComponentUniqueName).value_or("");
+      component_type = actuator.getString(EnergyManagementSystem_ActuatorFields::ActuatedComponentType).value_or("");
+      control_type = actuator.getString(EnergyManagementSystem_ActuatorFields::ActuatedComponentControlType).value_or("");
     } else {
       throw std::runtime_error(fmt::format("Error creating AlfalfaActuator: {} is not a supported object type", idd_type.valueDescription()));
     }
 
-    if (!component_name.is_initialized() || component_name.get().size() == 0) {
+    if (component_name.empty()) {
       throw std::runtime_error("Error creating AlfalfaActuator: Object is missing a component name");
     }
-    if (!component_type.is_initialized() || component_type.get().size() == 0) {
+    if (component_type.empty()) {
       throw std::runtime_error("Error creating AlfalfaActuator: Object is missing a component type");
     }
-    if (!control_type.is_initialized() || control_type.get().size() == 0) {
+    if (control_type.empty()) {
       throw std::runtime_error("Error creating AlfalfaActuator: Object is missing a control type");
     }
 
-    parameters["component_name"] = component_name.get();
-    parameters["component_type"] = component_type.get();
-    parameters["control_type"] = control_type.get();
+    parameters["component_name"] = component_name;
+    parameters["component_type"] = component_type;
+    parameters["control_type"] = control_type;
   }
 }  // namespace alfalfa
 }  // namespace openstudio
diff --git a/src/utilities/alfalfa/AlfalfaActuator.hpp b/src/utilities/alfalfa/AlfalfaActuator.hpp
index 0c9fa8ca1f1..1f5a1cfeef6 100644
--- a/src/utilities/alfalfa/AlfalfaActuator.hpp
+++ b/src/utilities/alfalfa/AlfalfaActuator.hpp
@@ -2,8 +2,7 @@
 #define ALFALFA_COMPONENT_ACTUATOR_HPP
 
 #include "AlfalfaComponent.hpp"
-#include "utilities/idf/IdfObject.hpp"
-#include "utilities/core/Logger.hpp"
+#include "../idf/IdfObject.hpp"
 #include "../UtilitiesAPI.hpp"
 
 namespace openstudio {
diff --git a/src/utilities/alfalfa/AlfalfaComponent.cpp b/src/utilities/alfalfa/AlfalfaComponent.cpp
index 262d628789c..07a979de26a 100644
--- a/src/utilities/alfalfa/AlfalfaComponent.cpp
+++ b/src/utilities/alfalfa/AlfalfaComponent.cpp
@@ -1,10 +1,9 @@
 #include "AlfalfaComponent.hpp"
-#include <iostream>
 
 namespace openstudio {
 namespace alfalfa {
 
-  AlfalfaComponent::AlfalfaComponent(const std::string& type, const Capability capabilities) : type(type), capabilities(capabilities) {}
+  AlfalfaComponent::AlfalfaComponent(const std::string& type, Capability capabilities) : type(type), capabilities(capabilities) {}
 
   bool AlfalfaComponent::canInput() const {
     return capabilities & Capability::Input;
@@ -15,7 +14,7 @@ namespace alfalfa {
   }
 
   bool AlfalfaComponent::operator==(const AlfalfaComponent& rhs) const {
-    return type == rhs.type && capabilities == rhs.capabilities && parameters == rhs.parameters;
+    return (type == rhs.type) && (capabilities == rhs.capabilities) && (parameters == rhs.parameters);
   }
 
 }  // namespace alfalfa
diff --git a/src/utilities/alfalfa/AlfalfaComponent.hpp b/src/utilities/alfalfa/AlfalfaComponent.hpp
index 223b6859dfb..efaaa881e29 100644
--- a/src/utilities/alfalfa/AlfalfaComponent.hpp
+++ b/src/utilities/alfalfa/AlfalfaComponent.hpp
@@ -24,7 +24,7 @@ namespace alfalfa {
   class UTILITIES_API AlfalfaComponent
   {
    public:
-    AlfalfaComponent(const std::string& type, const Capability capabilities);
+    AlfalfaComponent(const std::string& type, Capability capabilities);
 
     /**
      * Get if the component can be used as an Input to the model
diff --git a/src/utilities/alfalfa/AlfalfaConstant.hpp b/src/utilities/alfalfa/AlfalfaConstant.hpp
index 88197e8090f..dbafb99ce39 100644
--- a/src/utilities/alfalfa/AlfalfaConstant.hpp
+++ b/src/utilities/alfalfa/AlfalfaConstant.hpp
@@ -2,7 +2,6 @@
 #define ALFALFA_COMPONENT_CONSTANT_HPP
 
 #include "AlfalfaComponent.hpp"
-#include "utilities/core/Logger.hpp"
 
 #include "../UtilitiesAPI.hpp"
 
diff --git a/src/utilities/alfalfa/AlfalfaGlobalVariable.cpp b/src/utilities/alfalfa/AlfalfaGlobalVariable.cpp
index 9d2654cbe2a..d5054e70023 100644
--- a/src/utilities/alfalfa/AlfalfaGlobalVariable.cpp
+++ b/src/utilities/alfalfa/AlfalfaGlobalVariable.cpp
@@ -1,7 +1,7 @@
 #include "AlfalfaGlobalVariable.hpp"
 
-#include "utilities/idd/IddObject.hpp"
-#include "utilities/idd/IddEnums.hpp"
+#include "../idd/IddObject.hpp"
+#include "../idd/IddEnums.hpp"
 
 #include <utilities/idd/EnergyManagementSystem_GlobalVariable_FieldEnums.hxx>
 #include <utilities/idd/OS_EnergyManagementSystem_GlobalVariable_FieldEnums.hxx>
@@ -13,7 +13,7 @@ namespace openstudio {
 namespace alfalfa {
   AlfalfaGlobalVariable::AlfalfaGlobalVariable(const std::string& variable_name)
     : AlfalfaComponent("GlobalVariable", Capability::Output | Capability::Input) {
-    if (variable_name.size() == 0) {
+    if (variable_name.empty()) {
       throw std::runtime_error("Error creating AlfalfaGlobalVariable: variable_name must be non-empty");
     }
     parameters["variable_name"] = variable_name;
@@ -32,7 +32,7 @@ namespace alfalfa {
       throw std::runtime_error(fmt::format("Error creating AlfalfaGlobalVariable: {} is not a supported object type", idd_type.valueDescription()));
     }
 
-    if (!variable_name.is_initialized() || variable_name.get().size() == 0) {
+    if (!variable_name.is_initialized() || variable_name.get().empty()) {
       throw std::runtime_error("Error creating AlfalfaGlobalVariable: Object is missing a variable name");
     }
 
diff --git a/src/utilities/alfalfa/AlfalfaGlobalVariable.hpp b/src/utilities/alfalfa/AlfalfaGlobalVariable.hpp
index 193c1676fd5..a2e1e6fdcd2 100644
--- a/src/utilities/alfalfa/AlfalfaGlobalVariable.hpp
+++ b/src/utilities/alfalfa/AlfalfaGlobalVariable.hpp
@@ -2,8 +2,7 @@
 #define ALFALFA_COMPONENT_GLOBALVARIABLE_HPP
 
 #include "AlfalfaComponent.hpp"
-#include "utilities/idf/IdfObject.hpp"
-#include "utilities/core/Logger.hpp"
+#include "../idf/IdfObject.hpp"
 
 #include "../UtilitiesAPI.hpp"
 
diff --git a/src/utilities/alfalfa/AlfalfaMeter.cpp b/src/utilities/alfalfa/AlfalfaMeter.cpp
index daebe73722d..d9143df44fa 100644
--- a/src/utilities/alfalfa/AlfalfaMeter.cpp
+++ b/src/utilities/alfalfa/AlfalfaMeter.cpp
@@ -1,7 +1,7 @@
 #include "AlfalfaMeter.hpp"
 
-#include "utilities/idd/IddObject.hpp"
-#include "utilities/idd/IddEnums.hpp"
+#include "../idd/IddObject.hpp"
+#include "../idd/IddEnums.hpp"
 
 #include <utilities/idd/OS_Output_Meter_FieldEnums.hxx>
 #include <utilities/idd/Output_Meter_FieldEnums.hxx>
@@ -12,7 +12,7 @@
 namespace openstudio {
 namespace alfalfa {
   AlfalfaMeter::AlfalfaMeter(const std::string& meter_name) : AlfalfaComponent("Meter", Capability::Output) {
-    if (meter_name.size() == 0) {
+    if (meter_name.empty()) {
       throw std::runtime_error("Error creating AlfalfaMeter: meter_name must be non-empty");
     }
     parameters["meter_name"] = meter_name;
@@ -29,7 +29,7 @@ namespace alfalfa {
       throw std::runtime_error(fmt::format("Error creating AlfalfaMeter: {} is not a supported object type", idd_type.valueDescription()));
     }
 
-    if (!meter_name.is_initialized() || meter_name.get().size() == 0) {
+    if (!meter_name.is_initialized() || meter_name.get().empty()) {
       throw std::runtime_error("Error creating AlfalfaMeter: Object is missing a meter name");
     }
 
diff --git a/src/utilities/alfalfa/AlfalfaMeter.hpp b/src/utilities/alfalfa/AlfalfaMeter.hpp
index 396242759e1..68aa29d80f0 100644
--- a/src/utilities/alfalfa/AlfalfaMeter.hpp
+++ b/src/utilities/alfalfa/AlfalfaMeter.hpp
@@ -2,8 +2,7 @@
 #define ALFALFA_COMPONENT_METER_HPP
 
 #include "AlfalfaComponent.hpp"
-#include "utilities/idf/IdfObject.hpp"
-#include "utilities/core/Logger.hpp"
+#include "../idf/IdfObject.hpp"
 
 #include "../UtilitiesAPI.hpp"
 
diff --git a/src/utilities/alfalfa/AlfalfaOutputVariable.cpp b/src/utilities/alfalfa/AlfalfaOutputVariable.cpp
index 5aa88e662ec..6079eb2e87f 100644
--- a/src/utilities/alfalfa/AlfalfaOutputVariable.cpp
+++ b/src/utilities/alfalfa/AlfalfaOutputVariable.cpp
@@ -1,7 +1,7 @@
 #include "AlfalfaOutputVariable.hpp"
 
-#include "utilities/idd/IddObject.hpp"
-#include "utilities/idd/IddEnums.hpp"
+#include "../idd/IddObject.hpp"
+#include "../idd/IddEnums.hpp"
 
 #include <utilities/idd/OS_EnergyManagementSystem_OutputVariable_FieldEnums.hxx>
 #include <utilities/idd/OS_Output_Variable_FieldEnums.hxx>
@@ -15,9 +15,9 @@ namespace openstudio {
 namespace alfalfa {
   AlfalfaOutputVariable::AlfalfaOutputVariable(const std::string& variable_key, const std::string& variable_name)
     : AlfalfaComponent("OutputVariable", Capability::Output) {
-    if (variable_key.size() == 0) {
+    if (variable_key.empty()) {
       throw std::runtime_error("Error creating AlfalfaOutputVariable: variable_key must be non-empty");
-    } else if (variable_name.size() == 0) {
+    } else if (variable_name.empty()) {
       throw std::runtime_error("Error creating AlfalfaOutputVariable: variable_name must be non-empty");
     } else if (variable_key == "*") {
       throw std::runtime_error("Error creating AlfalfaOutputVariable: Key must not be wildcard '*'");
@@ -50,10 +50,10 @@ namespace alfalfa {
       throw std::runtime_error(fmt::format("Error creating AlfalfaOutputVariable: {} is not a supported object type", idd_type.valueDescription()));
     }
 
-    if (!variable_key.is_initialized() || variable_key.get().size() == 0) {
+    if (!variable_key.is_initialized() || variable_key.get().empty()) {
       throw std::runtime_error("Error creating AlfalfaOutputVariable: Object is missing a variable key");
     }
-    if (!variable_name.is_initialized() || variable_name.get().size() == 0) {
+    if (!variable_name.is_initialized() || variable_name.get().empty()) {
       throw std::runtime_error("Error creating AlfalfaOutputVariable: Object is missing a variable name");
     }
     if (variable_key.get() == "*") {
diff --git a/src/utilities/alfalfa/AlfalfaOutputVariable.hpp b/src/utilities/alfalfa/AlfalfaOutputVariable.hpp
index e0da631f4a1..e15d33699e2 100644
--- a/src/utilities/alfalfa/AlfalfaOutputVariable.hpp
+++ b/src/utilities/alfalfa/AlfalfaOutputVariable.hpp
@@ -2,8 +2,7 @@
 #define ALFALFA_COMPONENT_OUTPUTVARIABLE_HPP
 
 #include "AlfalfaComponent.hpp"
-#include "utilities/idf/IdfObject.hpp"
-#include "utilities/core/Logger.hpp"
+#include "../idf/IdfObject.hpp"
 
 #include "../UtilitiesAPI.hpp"
 
diff --git a/src/utilities/alfalfa/AlfalfaPoint.cpp b/src/utilities/alfalfa/AlfalfaPoint.cpp
index 24516e18e70..80ed30dc7aa 100644
--- a/src/utilities/alfalfa/AlfalfaPoint.cpp
+++ b/src/utilities/alfalfa/AlfalfaPoint.cpp
@@ -4,15 +4,21 @@
 #include <boost/regex.hpp>
 #include <fmt/format.h>
 
+#include <memory>
+#include <string_view>
+
 namespace openstudio {
 namespace alfalfa {
+
+  static constexpr std::string_view ID_VALID_CHARS_MSG = "IDs can only contain letters, numbers, and the following special characters _-[]():";
+
   namespace detail {
-    AlfalfaPoint_Impl::AlfalfaPoint_Impl() {}
+    AlfalfaPoint_Impl::AlfalfaPoint_Impl() = default;
   }  // namespace detail
 
-  AlfalfaPoint::AlfalfaPoint() : m_impl(std::shared_ptr<detail::AlfalfaPoint_Impl>(new detail::AlfalfaPoint_Impl())) {}
+  AlfalfaPoint::AlfalfaPoint() : m_impl(std::make_shared<detail::AlfalfaPoint_Impl>()) {}
 
-  AlfalfaPoint::AlfalfaPoint(const std::string& display_name) : m_impl(std::shared_ptr<detail::AlfalfaPoint_Impl>(new detail::AlfalfaPoint_Impl())) {
+  AlfalfaPoint::AlfalfaPoint(const std::string& display_name) : m_impl(std::make_shared<detail::AlfalfaPoint_Impl>()) {
     setDisplayName(display_name);
   }
 
@@ -65,7 +71,7 @@ namespace alfalfa {
     if (isValidId(id)) {
       m_impl->m_id = id;
     } else {
-      throw std::runtime_error(ID_VALID_CHARS_MSG);
+      throw std::runtime_error(ID_VALID_CHARS_MSG.data());
     }
   }
 
@@ -74,7 +80,7 @@ namespace alfalfa {
   }
 
   void AlfalfaPoint::setDisplayName(const std::string& display_name) {
-    if (display_name.size() == 0) {
+    if (display_name.empty()) {
       throw std::runtime_error("Display name must have non-zero length");
     }
     m_impl->m_display_name = display_name;
@@ -97,24 +103,24 @@ namespace alfalfa {
   Json::Value AlfalfaPoint::toJSON() const {
     Json::Value point;
 
-    if (id().is_initialized()) {
-      point["id"] = id().get();
+    if (auto id_ = id()) {
+      point["id"] = *id_;
     } else {
-      throw std::runtime_error("Point requires a valid ID for export. " + ID_VALID_CHARS_MSG);
+      throw std::runtime_error(fmt::format("Point requires a valid ID for export. {}", ID_VALID_CHARS_MSG));
     }
     point["name"] = displayName();
-    if (input().is_initialized()) {
-      point["input"]["type"] = input().get().type;
-      point["input"]["parameters"] = input().get().parameters;
+    if (auto input_ = input()) {
+      point["input"]["type"] = input_->type;
+      point["input"]["parameters"] = input_->parameters;
     }
 
-    if (output().is_initialized()) {
-      point["output"]["type"] = output().get().type;
-      point["output"]["parameters"] = output().get().parameters;
+    if (auto output_ = output()) {
+      point["output"]["type"] = output_->type;
+      point["output"]["parameters"] = output_->parameters;
     }
 
-    if (units().is_initialized()) {
-      point["units"] = units().get();
+    if (auto units_ = units()) {
+      point["units"] = *units_;
     }
 
     point["optional"] = optional();
@@ -122,11 +128,11 @@ namespace alfalfa {
     return point;
   }
 
-  bool AlfalfaPoint::isValidId(const std::string& id) const {
-    return boost::regex_match(id, boost::regex("^[A-Za-z0-9_\\-\\[\\]:()]*$")) && id.size() > 0;
+  bool AlfalfaPoint::isValidId(const std::string& id) {
+    return !id.empty() && boost::regex_match(id, boost::regex(R"(^[A-Za-z0-9_\-\[\]:()]*$)"));
   }
 
-  std::string AlfalfaPoint::toIdString(const std::string& str) const {
+  std::string AlfalfaPoint::toIdString(const std::string& str) {
     return boost::regex_replace(str, boost::regex(" "), "_");
   }
 
diff --git a/src/utilities/alfalfa/AlfalfaPoint.hpp b/src/utilities/alfalfa/AlfalfaPoint.hpp
index 2b4f6289cbf..9b6c394bf06 100644
--- a/src/utilities/alfalfa/AlfalfaPoint.hpp
+++ b/src/utilities/alfalfa/AlfalfaPoint.hpp
@@ -7,8 +7,8 @@
 #define ALFALFA_POINT_HPP
 
 #include "AlfalfaComponent.hpp"
-#include "utilities/core/Optional.hpp"
-#include "utilities/core/Logger.hpp"
+#include "../core/Optional.hpp"
+#include "../core/Logger.hpp"
 
 #include "../UtilitiesAPI.hpp"
 
@@ -96,11 +96,9 @@ namespace alfalfa {
    private:
     std::shared_ptr<detail::AlfalfaPoint_Impl> m_impl;
 
-    std::string toIdString(const std::string& str) const;
+    static std::string toIdString(const std::string& str);
 
-    bool isValidId(const std::string& id) const;
-
-    inline static const std::string ID_VALID_CHARS_MSG = "IDs can only contain letters, numbers, and the following special characters _-[]():";
+    static bool isValidId(const std::string& id);
 
     // configure logging
     REGISTER_LOGGER("openstudio.AlfalfaPoint")
diff --git a/src/utilities/alfalfa/AlfalfaPoint_Impl.hpp b/src/utilities/alfalfa/AlfalfaPoint_Impl.hpp
index 85bcb309325..610d5b1f269 100644
--- a/src/utilities/alfalfa/AlfalfaPoint_Impl.hpp
+++ b/src/utilities/alfalfa/AlfalfaPoint_Impl.hpp
@@ -17,10 +17,13 @@ namespace alfalfa {
       AlfalfaPoint_Impl();
 
      protected:
+      // TODO: is this pImpl is needed? If so, add getters and setters and make all members private.
       friend class openstudio::alfalfa::AlfalfaPoint;
-      boost::optional<AlfalfaComponent> m_input, m_output;
+      boost::optional<AlfalfaComponent> m_input;
+      boost::optional<AlfalfaComponent> m_output;
       std::string m_display_name;
-      boost::optional<std::string> m_units, m_id;
+      boost::optional<std::string> m_units;
+      boost::optional<std::string> m_id;
       bool m_optional = true;
     };
   }  // namespace detail
diff --git a/src/utilities/filetypes/AlfalfaJSON.cpp b/src/utilities/filetypes/AlfalfaJSON.cpp
index c9a43a06ca3..ffe6cde0d82 100644
--- a/src/utilities/filetypes/AlfalfaJSON.cpp
+++ b/src/utilities/filetypes/AlfalfaJSON.cpp
@@ -1,13 +1,15 @@
 #include "AlfalfaJSON.hpp"
 #include "AlfalfaJSON_Impl.hpp"
-#include "utilities/alfalfa/AlfalfaActuator.hpp"
-#include "utilities/alfalfa/AlfalfaOutputVariable.hpp"
-#include "utilities/alfalfa/AlfalfaGlobalVariable.hpp"
-#include "utilities/alfalfa/AlfalfaMeter.hpp"
-#include "utilities/alfalfa/AlfalfaConstant.hpp"
+#include "../alfalfa/AlfalfaActuator.hpp"
+#include "../alfalfa/AlfalfaOutputVariable.hpp"
+#include "../alfalfa/AlfalfaGlobalVariable.hpp"
+#include "../alfalfa/AlfalfaMeter.hpp"
+#include "../alfalfa/AlfalfaConstant.hpp"
 #include "../core/PathHelpers.hpp"
-#include "utilities/idd/IddObject.hpp"
-#include "utilities/idd/IddEnums.hpp"
+#include "../idd/IddObject.hpp"
+#include "../idd/IddEnums.hpp"
+#include "../../model/ModelObject.hpp"
+#include "../../model/ModelObject_Impl.hpp"
 
 #include <utilities/idd/OS_EnergyManagementSystem_OutputVariable_FieldEnums.hxx>
 #include <utilities/idd/OS_EnergyManagementSystem_GlobalVariable_FieldEnums.hxx>
@@ -21,15 +23,20 @@
 #include <utilities/idd/Output_Meter_FieldEnums.hxx>
 #include <utilities/idd/IddEnums.hxx>
 
-#include <iostream>
+#include <memory>
+
 namespace openstudio {
 namespace alfalfa {
   namespace detail {
-    AlfalfaJSON_Impl::AlfalfaJSON_Impl() {}
+    AlfalfaJSON_Impl::AlfalfaJSON_Impl() = default;
 
-    AlfalfaJSON_Impl::AlfalfaJSON_Impl(const std::string& s) {}
+    AlfalfaJSON_Impl::AlfalfaJSON_Impl(const std::string& s) {
+      // TODO: use s
+    }
 
-    AlfalfaJSON_Impl::AlfalfaJSON_Impl(const openstudio::path& p) {}
+    AlfalfaJSON_Impl::AlfalfaJSON_Impl(const openstudio::path& p) {
+      // TODO: use p
+    }
 
     void AlfalfaJSON_Impl::exposePoint(const AlfalfaPoint& point) {
       m_points.push_back(point);
@@ -68,7 +75,7 @@ namespace alfalfa {
 
     Json::Value AlfalfaJSON_Impl::toJSON() const {
       Json::Value root;
-      for (AlfalfaPoint point : m_points) {
+      for (const auto& point : m_points) {
         // No guard here as the toJSON call will throw an exception if the id does not exist.
         root[point.id().get()] = point.toJSON();
       }
@@ -76,37 +83,32 @@ namespace alfalfa {
     }
   }  // namespace detail
 
-  AlfalfaJSON::AlfalfaJSON() : m_impl(std::shared_ptr<detail::AlfalfaJSON_Impl>(new detail::AlfalfaJSON_Impl())) {}
+  AlfalfaJSON::AlfalfaJSON() : m_impl(std::make_shared<detail::AlfalfaJSON_Impl>()) {}
 
-  AlfalfaJSON::AlfalfaJSON(const std::string& s) : m_impl(std::shared_ptr<detail::AlfalfaJSON_Impl>(new detail::AlfalfaJSON_Impl(s))) {}
+  AlfalfaJSON::AlfalfaJSON(const std::string& s) : m_impl(std::make_shared<detail::AlfalfaJSON_Impl>(s)) {}
 
-  AlfalfaJSON::AlfalfaJSON(const openstudio::path& p) : m_impl(std::shared_ptr<detail::AlfalfaJSON_Impl>(new detail::AlfalfaJSON_Impl(p))) {}
+  AlfalfaJSON::AlfalfaJSON(const openstudio::path& p) : m_impl(std::make_shared<detail::AlfalfaJSON_Impl>(p)) {}
 
   boost::optional<AlfalfaPoint> AlfalfaJSON::exposeConstant(float value, const std::string& display_name) {
-    AlfalfaConstant component(value);
-    return exposeFromComponent(component, display_name);
+    return exposeFromComponent(AlfalfaConstant{value}, display_name);
   }
 
   boost::optional<AlfalfaPoint> AlfalfaJSON::exposeMeter(const std::string& meter_name, const std::string& display_name) {
-    AlfalfaMeter component(meter_name);
-    return exposeFromComponent(component, display_name);
+    return exposeFromComponent(AlfalfaMeter{meter_name}, display_name);
   }
 
   boost::optional<AlfalfaPoint> AlfalfaJSON::exposeActuator(const std::string& component_name, const std::string& component_type,
                                                             const std::string& control_type, const std::string& display_name) {
-    AlfalfaActuator component(component_name, component_type, control_type);
-    return exposeFromComponent(component, display_name);
+    return exposeFromComponent(AlfalfaActuator{component_name, component_type, control_type}, display_name);
   }
 
   boost::optional<AlfalfaPoint> AlfalfaJSON::exposeOutputVariable(const std::string& variable_key, const std::string& variable_name,
                                                                   const std::string& display_name) {
-    AlfalfaOutputVariable component(variable_key, variable_name);
-    return exposeFromComponent(component, display_name);
+    return exposeFromComponent(AlfalfaOutputVariable{variable_key, variable_name}, display_name);
   }
 
   boost::optional<AlfalfaPoint> AlfalfaJSON::exposeGlobalVariable(const std::string& variable_name, const std::string& display_name) {
-    AlfalfaGlobalVariable component(variable_name);
-    return exposeFromComponent(component, display_name);
+    return exposeFromComponent(AlfalfaGlobalVariable{variable_name}, display_name);
   }
 
   void AlfalfaJSON::exposePoint(const AlfalfaPoint& point) {
@@ -115,35 +117,34 @@ namespace alfalfa {
 
   boost::optional<AlfalfaPoint> AlfalfaJSON::exposeFromObject(const openstudio::IdfObject& idf_object, const std::string& display_name) {
     IddObjectType idd_type = idf_object.iddObject().type();
-    std::string _display_name = display_name;
+    std::string display_name_ = display_name;
 
-    if (display_name.size() == 0) {
-      boost::optional<std::string> name = getName(idf_object);
-      if (name.is_initialized()) {
-        _display_name = name.get();
+    if (display_name.empty()) {
+      if (auto name_ = getName(idf_object)) {
+        display_name_ = std::move(*name_);
       }
     }
 
-    AlfalfaComponent* component;
+    boost::optional<AlfalfaComponent> component;
     try {
       switch (idd_type.value()) {
         case IddObjectType::OS_Output_Meter:
         case IddObjectType::Output_Meter:
-          component = new AlfalfaMeter(idf_object);
+          component = AlfalfaMeter(idf_object);
           break;
         case IddObjectType::OS_EnergyManagementSystem_Actuator:
         case IddObjectType::EnergyManagementSystem_Actuator:
-          component = new AlfalfaActuator(idf_object);
+          component = AlfalfaActuator(idf_object);
           break;
         case IddObjectType::OS_Output_Variable:
         case IddObjectType::Output_Variable:
         case IddObjectType::EnergyManagementSystem_OutputVariable:
         case IddObjectType::OS_EnergyManagementSystem_OutputVariable:
-          component = new AlfalfaOutputVariable(idf_object);
+          component = AlfalfaOutputVariable(idf_object);
           break;
         case IddObjectType::OS_EnergyManagementSystem_GlobalVariable:
         case IddObjectType::EnergyManagementSystem_GlobalVariable:
-          component = new AlfalfaGlobalVariable(idf_object);
+          component = AlfalfaGlobalVariable(idf_object);
           break;
         default:
           LOG(Error, "Unable to create Alfalfa Point from Object of type " + idd_type.valueDescription());
@@ -153,24 +154,24 @@ namespace alfalfa {
       LOG(Error, error.what());
       return boost::none;
     }
-    return exposeFromComponent(*component, _display_name);
+    return exposeFromComponent(*component, display_name_);
   }
 
   boost::optional<AlfalfaPoint> AlfalfaJSON::exposeFromComponent(const AlfalfaComponent& component, const std::string& display_name) {
-    std::string _display_name = display_name;
-    if (display_name.size() == 0) {
+    std::string display_name_ = display_name;
+    if (display_name.empty()) {
       if (component.type == "Actuator") {
-        _display_name = "Actuator for " + component.parameters["component_name"].asString() + ":" + component.parameters["component_type"].asString()
+        display_name_ = "Actuator for " + component.parameters["component_name"].asString() + ":" + component.parameters["component_type"].asString()
                         + ":" + component.parameters["control_type"].asString();
       } else if (component.type == "Constant") {
         LOG(Error, "Constant points must be provided with a display name");
         return boost::none;
       } else if (component.type == "Meter") {
-        _display_name = "Meter for " + component.parameters["meter_name"].asString();
+        display_name_ = "Meter for " + component.parameters["meter_name"].asString();
       } else if (component.type == "GlobalVariable") {
-        _display_name = "Global Variable for " + component.parameters["variable_name"].asString();
+        display_name_ = "Global Variable for " + component.parameters["variable_name"].asString();
       } else if (component.type == "OutputVariable") {
-        _display_name =
+        display_name_ =
           "Output Variable for " + component.parameters["variable_key"].asString() + ":" + component.parameters["variable_name"].asString();
       } else {
         LOG(Error, "Invalid component type");
@@ -178,7 +179,7 @@ namespace alfalfa {
       }
     }
 
-    AlfalfaPoint point(_display_name);
+    AlfalfaPoint point(display_name_);
     if (component.canInput()) {
       point.setInput(component);
     }
@@ -186,19 +187,19 @@ namespace alfalfa {
       point.setOutput(component);
     }
     exposePoint(point);
-    boost::optional<AlfalfaPoint> result = point;
-    return result;
+    return {std::move(point)};
   }
 
   boost::optional<std::string> AlfalfaJSON::getName(const openstudio::IdfObject& idf_object) {
-    if (dynamic_cast<const openstudio::model::ModelObject*>(&idf_object) != nullptr) {
-      boost::optional<std::string> display_name = dynamic_cast<const openstudio::model::ModelObject*>(&idf_object)->displayName();
-      if (display_name.is_initialized()) {
+    if (auto mo_ = idf_object.optionalCast<model::ModelObject>()) {
+      if (boost::optional<std::string> display_name = mo_->displayName()) {
         return display_name;
       }
     }
-    if (idf_object.name().is_initialized() && idf_object.name().get().size() > 0) {
-      return idf_object.name();
+    if (auto name_ = idf_object.name()) {
+      if (!name_->empty()) {
+        return name_;
+      }
     }
     return boost::none;
   }
diff --git a/src/utilities/filetypes/AlfalfaJSON.hpp b/src/utilities/filetypes/AlfalfaJSON.hpp
index ed40d5741d8..62848b93748 100644
--- a/src/utilities/filetypes/AlfalfaJSON.hpp
+++ b/src/utilities/filetypes/AlfalfaJSON.hpp
@@ -14,8 +14,7 @@
 #include "../core/Path.hpp"
 #include "../core/Logger.hpp"
 #include "../core/Optional.hpp"
-
-#include "model/ModelObject.hpp"
+#include "../idf/IdfObject.hpp"
 
 namespace openstudio {
 namespace alfalfa {
@@ -100,7 +99,7 @@ namespace alfalfa {
     std::vector<AlfalfaPoint> getPoints();
 
    private:
-    boost::optional<std::string> getName(const openstudio::IdfObject& idf_object);
+    static boost::optional<std::string> getName(const openstudio::IdfObject& idf_object);
 
     std::shared_ptr<detail::AlfalfaJSON_Impl> m_impl;
 
diff --git a/src/utilities/filetypes/AlfalfaJSON_Impl.hpp b/src/utilities/filetypes/AlfalfaJSON_Impl.hpp
index 0f67b4d7136..60acd773b10 100644
--- a/src/utilities/filetypes/AlfalfaJSON_Impl.hpp
+++ b/src/utilities/filetypes/AlfalfaJSON_Impl.hpp
@@ -3,8 +3,8 @@
 
 #include "../UtilitiesAPI.hpp"
 
-#include "../core/Path.hpp"
 #include "../alfalfa/AlfalfaPoint.hpp"
+#include "../core/Path.hpp"
 
 #include <vector>
 #include <json/json.h>


Test patch:
diff --git a/python/engine/test/PythonEngine_GTest.cpp b/python/engine/test/PythonEngine_GTest.cpp
index 9c42d058522..20a603aecc3 100644
--- a/python/engine/test/PythonEngine_GTest.cpp
+++ b/python/engine/test/PythonEngine_GTest.cpp
@@ -145,28 +145,28 @@ TEST_F(PythonEngineFixture, AlfalfaMeasure) {
 
   ASSERT_EQ(measurePtr->name(), "Alfalfa Measure");
 
-  std::string workflow_json = "\
-  {\
-    \"seed\": \"../seed.osm\",\
-    \"weather_file\": \"../weather.epw\",\
-    \"steps\": [\
-      {\
-        \"arguments\": {},\
-        \"description\": \"The method attempts to build an alfalfa json in the measure\",\
-        \"measure_dir_name\": \"AlfalfaMeasure\",\
-        \"modeler_description\": \"The method attempts to build an alfalfa json in the measure\",\
-        \"name\": \"AlfalfaMeasure\"\
-      }\
-    ]\
-  }";
-
-
+  std::string workflow_json = R"json(
+{
+  "seed": "../seed.osm",
+  "weather_file": "../weather.epw",
+  "steps": [
+    {
+      "arguments": {},
+      "description": "The method attempts to build an alfalfa json in the measure",
+      "measure_dir_name": "AlfalfaMeasure",
+      "modeler_description": "The method attempts to build an alfalfa json in the measure",
+      "name": "AlfalfaMeasure"
+    }
+  ]
+}
+  )json";
 
   openstudio::model::Model model;
   openstudio::WorkflowJSON workflow = *openstudio::WorkflowJSON::load(workflow_json);
   openstudio::measure::OSRunner runner(workflow);
+  EXPECT_TRUE(runner.alfalfa().getPoints().empty());
 
   openstudio::measure::OSArgumentMap arguments;
   measurePtr->run(model, runner, arguments);
-
+  EXPECT_EQ(5, runner.alfalfa().getPoints().size());
 }