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

Fix #4885 - Rewrite MeasureManager in C++ #4920

Merged
merged 49 commits into from
Sep 28, 2023
Merged
Show file tree
Hide file tree
Changes from 44 commits
Commits
Show all changes
49 commits
Select commit Hold shift + click to select a range
742f682
WIP
jmarrec May 17, 2023
5f54972
WIP2
jmarrec Jun 13, 2023
fe0bf72
Implement getIdf
jmarrec Jun 13, 2023
7a0ef58
WIP implementing getAndUpdateMeasure inside MeasureManager with caching
jmarrec Jun 13, 2023
6d4d3c2
More WIP putting all the pieces together
jmarrec Jun 19, 2023
f505532
passing http://localhost was binding to tcp6, not tcp4, change to 0.0…
jmarrec Jun 20, 2023
7ddca5d
Use pytest to test the measure manager. Starting to look good!
jmarrec Jun 20, 2023
e436183
Add OSArgument::toJSON
jmarrec Jun 21, 2023
93b0b34
Add `measure_dir` to BCLMeasure/BCLXML toJSON
jmarrec Jun 21, 2023
dc9d8bd
Almost working! Ruby compute argument crashes...
jmarrec Jun 22, 2023
8bd9138
Avoid crashing when trying to get location when ruby engine eval failed
jmarrec Jun 22, 2023
bf70f1e
Add `MeasureType OSMeasure::measureType() const`
jmarrec Jun 22, 2023
6837b05
change the way to infer the ruby className because of a stack too dee…
jmarrec Jun 22, 2023
2d8bb96
Try to move inferClassName / loadMeasure at ScriptEngine level
jmarrec Jun 22, 2023
be003fa
Dry it up: Use the new methods, and in the `measure`subcommand use th…
jmarrec Jun 22, 2023
f59cf9f
Use conftest pytest from 9743a6be98f197de4a2823a71fa487ffefaee8e9
jmarrec Jun 23, 2023
2dbba58
Wrap the code that queries the measure in a try-catch after having it…
jmarrec Jun 23, 2023
7602d91
Add a module docstring in the pytest file + expanduser
jmarrec Jun 23, 2023
1e48a23
Add a dynamic monkeypatch to support old ReportingMeasures that didn'…
jmarrec Jun 23, 2023
3f0624f
Remove now-useless getAndUpdateMeasure in MeasureeUpdateCommand becau…
jmarrec Jun 23, 2023
ecae640
Add OSOutput::toJSON
jmarrec Jun 23, 2023
790a6f3
Generate README.md from ERB via C++
jmarrec Jun 26, 2023
7588d12
Need a pointer to avoid a copy, otherwise when clearing measureInfos …
jmarrec Jun 26, 2023
9ad3e3a
Debug a SystemStackError...
jmarrec Jun 26, 2023
4cf93a2
Try to unbreak windows when cpprestsdk uses wide strings
jmarrec Jun 28, 2023
83669aa
Oops, was included a cpp instead of hpp
jmarrec Jun 28, 2023
db32b90
Rely our own own toString(std::wstring&) not utility::conversions::to…
jmarrec Jun 28, 2023
6378003
Windows set hostname to localhost only otherwise it doesn't work
jmarrec Jun 28, 2023
7e4f358
Handle windows differences in paths in pytest
jmarrec Jun 28, 2023
3c4a1c3
Fix runtime errors in C++ MeasureManager Server
kbenne Aug 10, 2023
7c37fa0
Widden paths on windows when passing them to python.
jmarrec Aug 14, 2023
29d7726
Remove interrupt handler, not used anymore.
jmarrec Aug 14, 2023
eb1c3ca
Use a member function pointer instead of a std::function
jmarrec Aug 14, 2023
eaeed53
Add comments explaining the shenanigans done to ensure the **main** t…
jmarrec Aug 14, 2023
449c70c
Handle incorrect "set" my_measures_dir gracefully (wrong directory or…
jmarrec Aug 14, 2023
3d1bf85
Missing early return.
jmarrec Aug 14, 2023
b84a532
Move ThreadSafeDeque into its own header to avoid polluting all other…
jmarrec Aug 14, 2023
3e228f9
Merge remote-tracking branch 'origin/develop' into 4885_MeasureManage…
jmarrec Aug 14, 2023
9b31f65
Remove debug messages
jmarrec Aug 16, 2023
6343a48
/compute_arguments endpoint must take ONLY an OSM
jmarrec Aug 18, 2023
2a7c1cf
Forgot a debug message
jmarrec Aug 18, 2023
b58c33d
Add a ctest for running a Python measure FIRST then a ruby: crash on mac
jmarrec Aug 18, 2023
13cbc82
Add (and flush) a stdout message on MeasureManager ready
kbenne Sep 22, 2023
e1f5e0c
Merge branch '4885_MeasureManager_c++' of github.com:NREL/OpenStudio …
kbenne Sep 22, 2023
dd55a86
Update src/cli/MeasureManager.cpp
kbenne Sep 25, 2023
b2b5cf7
Fix runtime error for Python/Ruby workflows
kbenne Sep 26, 2023
109be9d
Merge branch '4885_MeasureManager_c++' of https://github.com/NREL/Ope…
kbenne Sep 26, 2023
f4d43bf
Fix fflush
jmarrec Sep 27, 2023
22d99c4
Merge branch 'develop' into 4885_MeasureManager_c++
jmarrec Sep 27, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,13 @@ clang_format.patch

.DS_Store
__pycache__/
*.pyc
*.pyc

# Unit test / coverage reports
htmlcov/
.coverage
.coverage.*
.cache
coverage.xml
.pytest_cache/
junit.xml
124 changes: 115 additions & 9 deletions python/engine/PythonEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,10 @@
#include <utilities/core/ApplicationPathHelpers.hpp>
#include "../../src/utilities/core/Filesystem.hpp"
#include <fmt/format.h>

#include <stdexcept>
#include <string>
#include <iostream>
#include <type_traits>

#ifdef __GNUC__
# pragma GCC diagnostic push
Expand All @@ -27,15 +28,31 @@
namespace openstudio {

void addToPythonPath(const openstudio::path& includePath) {
if (!includePath.empty()) {
PyObject* sys = PyImport_ImportModule("sys");
PyObject* sysPath = PyObject_GetAttrString(sys, "path");
Py_DECREF(sys); // PyImport_ImportModule returns a new reference, decrement it

// fmt::print("Prepending '{}' to sys.path\n", includePath);
PyObject* unicodeIncludePath = PyUnicode_FromString(includePath.string().c_str());
PyList_Insert(sysPath, 0, unicodeIncludePath);
Py_DECREF(sysPath); // PyObject_GetAttrString returns a new reference, decrement it
if (includePath.empty()) {
return;
}

// fmt::print("Prepending '{}' to sys.path\n", includePath);

PyObject* unicodeIncludePath = nullptr;
if constexpr (std::is_same_v<typename openstudio::path::value_type, wchar_t>) {
const std::wstring ws = includePath.generic_wstring();
unicodeIncludePath = PyUnicode_FromWideChar(ws.c_str(), static_cast<Py_ssize_t>(ws.size())); // New reference
} else {
const std::string s = includePath.generic_string();
unicodeIncludePath = PyUnicode_FromString(s.c_str()); // New reference
}

if (unicodeIncludePath == nullptr) {
throw std::runtime_error(fmt::format("Unable to convert path '{}' for addition to sys.path in Python", includePath.generic_string()));
}

PyObject* sysPath = PySys_GetObject("path"); // Borrowed reference
int ret = PyList_Insert(sysPath, 0, unicodeIncludePath);
Py_DECREF(unicodeIncludePath);
if (ret != 0) {
throw std::runtime_error(fmt::format("Unable to add path '{}' to the sys.path in Python", includePath.generic_string()));
}
}

Expand Down Expand Up @@ -242,6 +259,95 @@ void* PythonEngine::getAs_impl(ScriptObject& obj, const std::type_info& ti) {

return return_value;
}

std::string PythonEngine::inferMeasureClassName(const openstudio::path& measureScriptPath) {

auto inferClassNameCmd = fmt::format(R"python(
import importlib.util
import inspect
spec = importlib.util.spec_from_file_location(f"throwaway", "{}")

module = importlib.util.module_from_spec(spec)
spec.loader.exec_module(module)
class_members = inspect.getmembers(module, lambda x: inspect.isclass(x) and issubclass(x, openstudio.measure.OSMeasure))
assert len(class_members) == 1
measure_name, measure_typeinfo = class_members[0]
)python",
measureScriptPath.generic_string());

std::string className;
try {
exec(inferClassNameCmd);
ScriptObject measureClassNameObject = eval("measure_name");
className = *getAs<std::string*>(measureClassNameObject);
} catch (const std::runtime_error& e) {
auto msg = fmt::format("Failed to infer measure name from {}: {}", measureScriptPath.generic_string(), e.what());
fmt::print(stderr, "{}\n", msg);
}

return className;
}

// Ideally this would return a openstudio::measure::OSMeasure* or a shared_ptr<openstudio::measure::OSMeasure> but this poses memory management
// issue for the underlying ScriptObject (and VALUE or PyObject), so just return the ScriptObject
ScriptObject PythonEngine::loadMeasure(const openstudio::path& measureScriptPath, std::string_view className) {

ScriptObject result;

auto importCmd = fmt::format(R"python(
import importlib.util
spec = importlib.util.spec_from_file_location('{}', r'{}')
module = importlib.util.module_from_spec(spec)
spec.loader.exec_module(module)
)python",
className, measureScriptPath.generic_string());

// fmt::print("\nimportCmd:\n{}\n", importCmd);
try {
exec(importCmd);
} catch (const std::runtime_error& e) {
auto msg = fmt::format("Failed to load measure '{}' from '{}': {}", className, measureScriptPath.generic_string(), e.what());
fmt::print(stderr, "{}\n", msg);
}

try {
result = eval(fmt::format("module.{}()", className));
} catch (const std::runtime_error& e) {
auto msg = fmt::format("Failed to instantiate measure '{}' from '{}': {}", className, measureScriptPath.generic_string(), e.what());
fmt::print(stderr, "{}\n", msg);
}

return result;
}

int PythonEngine::numberOfArguments(ScriptObject& classInstanceObject, std::string_view methodName) {

int numberOfArguments = -1;

auto val = std::any_cast<PythonObject>(classInstanceObject.object);
if (PyObject_HasAttrString(val.obj_, methodName.data()) == 0) {
// FAILED
return numberOfArguments;
}

PyObject* method = PyObject_GetAttrString(val.obj_, methodName.data()); // New reference
if (PyMethod_Check(method)) {
PyObject* func = PyMethod_Function(method); // Borrowed
if (auto* code = PyFunction_GetCode(func)) { // Borrowed
auto* co = (PyCodeObject*)code;
numberOfArguments = co->co_argcount - 1; // This includes `self`
}
} else if (PyFunction_Check(method)) {
// Shouldn't enter this block here
if (auto code = PyFunction_GetCode(method)) {
auto* co = (PyCodeObject*)code;
numberOfArguments = co->co_argcount;
}
}
Py_DECREF(method);
return numberOfArguments;
}

} // namespace openstudio

extern "C"
Expand Down
6 changes: 6 additions & 0 deletions python/engine/PythonEngine.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,12 @@ class PythonEngine final : public ScriptEngine

virtual void setupPythonPath(const std::vector<openstudio::path>& includeDirs) override;

virtual std::string inferMeasureClassName(const openstudio::path& measureScriptPath) override;

virtual ScriptObject loadMeasure(const openstudio::path& measureScriptPath, std::string_view className) override;

virtual int numberOfArguments(ScriptObject& methodObject, std::string_view methodName) override;

protected:
void* getAs_impl(ScriptObject& obj, const std::type_info&) override;
void importOpenStudio();
Expand Down
22 changes: 22 additions & 0 deletions resources/Examples/compact_osw/compact_python_then_ruby.osw
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
{
"weather_file": "srrl_2013_amy.epw",
"seed_file": "seb.osm",
"steps": [
{
"measure_dir_name": "IncreaseRoofRValuePython",
"arguments": {
"r_value": 45
}
},
{
"measure_dir_name": "SetEplusInfiltration",
"arguments": {
"flowPerZoneFloorArea": 10.76
}
},
{
"measure_dir_name": "Standard Reports",
"arguments": {}
}
]
}
70 changes: 70 additions & 0 deletions ruby/bindings/InitRubyBindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,76 @@ module Ruleset
end

end # module Ruleset

module Measure
class RubyMeasureInfoBinding < OpenStudio::Measure::MeasureInfoBinding
def initialize
super() # needed
@info = nil
end
def setMeasureInfo(info)
@info = info
end
def measure_info
@info
end
def error
@info.error.to_s
end
def measureType
@info.measureType.valueName.to_s
end
def className
@info.className.to_s
end
def name
@info.name.to_s
end
def description
@info.description.to_s
end
def taxonomy
@info.taxonomy.to_s
end
def modelerDescription
@info.modelerDescription.to_s
end
def arguments
@info.arguments.map{|a| a.toJSON()}
end
def outputs
@info.outputs.map{|a| a.toJSON()}
end
def get_binding
result = binding()
return result
end
def renderFile(readme_in_path)
require 'erb'
begin
readme_out_path = File.join(File.dirname(readme_in_path), File.basename(readme_in_path, File.extname(readme_in_path)))
readme_in = File.read(readme_in_path)
renderer = ERB.new(readme_in)
readme_out = renderer.result(get_binding())
rescue
info = OpenStudio::Measure::OSMeasureInfo.new(e.message)
return false # @info
end
# write README.md file
File.open(readme_out_path, 'w') do |file|
file << readme_out
# make sure data is written to the disk one way or the other
begin
file.fsync
rescue StandardError
file.flush
end
end
return true # @info
end
end
end # module Measure

end # module OpenStudio

module OpenStudio
Expand Down
87 changes: 87 additions & 0 deletions ruby/engine/RubyEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

#include "RubyEngine.hpp"
#include "InitRubyBindings.hpp"
#include "RubyException.hpp"
#include <embedded_files.hxx>
#include <csignal>
#include <stdexcept>
Expand Down Expand Up @@ -118,6 +119,92 @@ void* RubyEngine::getAs_impl(ScriptObject& obj, const std::type_info& ti) {
return return_value;
}

std::string RubyEngine::inferMeasureClassName(const openstudio::path& measureScriptPath) {

auto inferClassNameCmd = fmt::format(R"ruby(
ObjectSpace.garbage_collect
ObjectSpace.garbage_collect
# Measure should be at root level (not inside a module) so we can just get constants
measurePath = '{}'
prev = Object.constants
# puts "prev = #{{prev}}"
load measurePath # need load in case have seen this script before
just_defined = Object.constants - prev
# puts "just_defined = #{{just_defined}}"
just_defined.select!{{|c| Object.const_get(c).ancestors.include?(OpenStudio::Measure::OSMeasure)}}
# puts "just_defined, filtered = #{{just_defined}}"

if just_defined.empty?
raise "Unable to extract OpenStudio::Measure::OSMeasure object from " +
measurePath + ". The script should contain a class that derives " +
"from OpenStudio::Measure::OSMeasure and should close with a line stating " +
"the class name followed by .new.registerWithApplication."
end
if just_defined.size > 1
raise "Found more than one OSMeasure at #{{measurePath}}: #{{just_defined}}"
end
c = just_defined[0]
class_info = Object.const_get(c)
$measure_name = class_info.to_s

# Undef what we loaded
just_defined.each {{|x| Object.send(:remove_const, x) }}
ObjectSpace.garbage_collect
ObjectSpace.garbage_collect
)ruby",
measureScriptPath.generic_string());

std::string className;

try {
exec(inferClassNameCmd);
ScriptObject measureClassNameObject = eval("$measure_name");
// measureClassNameObject = rubyEngine->eval(fmt::format("{}.new()", className));
// ScriptObject measureClassNameObject = rubyEngine->eval(inferClassName);
className = *getAs<std::string*>(measureClassNameObject);
} catch (const RubyException& e) {
auto msg = fmt::format("Failed to infer measure name from {}: {}\nlocation={}", measureScriptPath.generic_string(), e.what(), e.location());
fmt::print(stderr, "{}\n", msg);
}

return className;
}

// Ideally this would return a openstudio::measure::OSMeasure* or a shared_ptr<openstudio::measure::OSMeasure> but this poses memory management
// issue for the underlying ScriptObject (and VALUE or PyObject), so just return the ScriptObject
ScriptObject RubyEngine::loadMeasure(const openstudio::path& measureScriptPath, std::string_view className) {

ScriptObject result;
try {
auto importCmd = fmt::format("load '{}'", measureScriptPath.generic_string());
exec(importCmd);
} catch (const RubyException& e) {
auto msg =
fmt::format("Failed to load measure '{}' from '{}': {}\nlocation={}", className, measureScriptPath.generic_string(), e.what(), e.location());
fmt::print(stderr, "{}\n", msg);
}
try {
result = eval(fmt::format("{}.new()", className));
} catch (const RubyException& e) {
auto msg = fmt::format("Failed to instantiate measure '{}' from '{}': {}\nlocation={}", className, measureScriptPath.generic_string(), e.what(),
e.location());
fmt::print(stderr, "{}\n", msg);
}
return result;
}

int RubyEngine::numberOfArguments(ScriptObject& methodObject, std::string_view methodName) {
auto val = std::any_cast<VALUE>(methodObject.object);
ID method_id = rb_intern(methodName.data());
// Ruby IS SO WEIRD. This will return n for methods that take a fixed number of arguments, and -n - 1 where n is the number of required arguments...
// def f() => 0
// def f(a) => 1
// def f(a = nil) => -1! Because WHY NOT, right?
// def f(a, c = nil) => -2
// def f(a, b = nil, c = nil) => also -2
return rb_obj_method_arity(val, method_id);
}

} // namespace openstudio

extern "C"
Expand Down
6 changes: 6 additions & 0 deletions ruby/engine/RubyEngine.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,12 @@ class RubyEngine final : public ScriptEngine
const openstudio::path& gemHomeDir, const openstudio::path& bundleGemFilePath,
const openstudio::path& bundleGemDirPath, const std::string& bundleWithoutGroups) override;

virtual std::string inferMeasureClassName(const openstudio::path& measureScriptPath) override;

virtual ScriptObject loadMeasure(const openstudio::path& measureScriptPath, std::string_view className) override;

virtual int numberOfArguments(ScriptObject& methodObject, std::string_view methodName) override;

protected:
// convert the underlying object to the correct type, then return it as a void *
// so the above template function can provide it back to the caller.
Expand Down
Loading
Loading