Allow model instead of workspace in `energyPlusOutputRequests`

[shorowit]

Enhancement Request

When writing a reporting measure and requesting outputs (using output variables like Output:Variable or other output configuration objects like OutputControl:Timestamps), you only have access to workspace and have to insert E+ objects. The initial reason for this may have been because the objects were not wrapped in the SDK, but almost all output-related objects are wrapped in OpenStudio these days.

So can we allow model instead of workspace for output requests? If it needs to be non-breaking, it could be a new method such that the existing method is preserved.

Pros:

1. It's cleaner, better, and more maintainable for a writer of a reporting measure to interact with OpenStudio objects (vs writing E+ objects as strings).
2. It allows the writer of a reporting measure to set object properties individually. With the E+ objects, you have to insert an entire valid object, and it's not obvious what behavior should be expected for unique objects that have already been assigned some values upstream.
3. The OS development team won't have to maintain a list of valid E+ objects that can be used (and write special code for some of them).

Cons:

1. A writer of a reporting measure might interact with the model in ways that are not desired/expected (and which have nothing to do with output).

There may be other pros/cons, but that's what immediately came to mind.

[joseph-robertson]

How about a new openstudioOutputRequests method that happens before FT? Assuming energyPlusOutputRequests happens after FT, this new method may be necessary?

Edit: perhaps the proper analogous method name would be modelOutputRequests.

[shorowit]

Interesting idea. That could make this a non-breaking enhancement.

[kbenne]

👍

[kbenne]

Then the next question is can we support this only for labs? It would make development easier. Maybe a little carrot for using labs?

[kbenne]

How about...

virtual Model modelOutputRequests(OSRunner& runner, const std::map<std::string, OSArgument>& user_arguments) const;

My thought was for the user defined measure to return a new OpenStudio Model with the desired output requests. The workflow would then merge the returned model with the worfklow model. This would allow us to choose only the allowed types to merge into the workflow model.

Of course as an alternative we could just pass the workflow model and let the Measure author do whatever they want, but then ModelMeasure and ReportingMeasure essentially merge into one thing.

virtual bool modelOutputRequests(OSRunner& runner, Model& model, const std::map<std::string, OSArgument>& user_arguments) const;

Anyone care to vote on these two options?

[shorowit]

I vote for the latter.

The line is blurry about what E+ objects a reporting measure might want to interact with. For example, if you wanted to report thermal comfort-related outputs, you might want to interact with the People object -- work efficiency schedule, clothing inputs, thermal comfort model, etc. It is frustrating when a software tool tries to be helpful but instead limits what a user wants to achieve.

Also, OS has not had a good history of adding support for new output-related objects - it still significantly lags behind right now. For example, the current workflow-gem only allows the Output:Table:SummaryReports unique object, whereas E+ has close to a dozen such Output:Foo or OutputControl:Foo objects.

By choosing a hands-off approach, OS reduces maintenance and increases flexibility.

[kbenne]

I agree that option one might be annoyingly (artificially) limited. Also true that option one creates a maintenance issue. My only hesitation about option two is that it effectively turns ReportingMeasure into a ModelMeasure. Maybe that is ok? Maybe the two types of Measures should just be merged into one thing?

@DavidGoldwasser @shorowit

[shorowit]

Maybe I'm just missing your point, but they still behave pretty differently in terms of when their methods get executed in a workflow.

ModelMeasure.run > ReportingMeasure.modelOutputRequests > FT > EnergyPlusMeasure.run > Simulation > ReportingMeasure.run

[kbenne]

I think ReportingMeasure.modelOutputRequests could do everything that ModelMeasure.run could do and since they are neighbors in the workflow sequence there wouldn't really be any difference. Like I said, maybe that is fine, but I think it may be true.

[kbenne]

If nobody else chimes in I think we'll go with option 2. I'll get over my slight concern about competing with ModelMeasure. It will probably look like this...

virtual bool modelOutputRequests(OSRunner& runner, Model& model, const std::map<std::string, OSArgument>& user_arguments) const;

[joseph-robertson]

@kbenne What do you mean by "Maybe the two types of Measures should just be merged into one thing?"?

[kbenne]

@joseph-robertson

if ReportingMeasure::run became ModelMeasure::createReports we would end up with about the same functionality all in ModelMeasure. I don't think I'm advocating for this. It would be a huge breaking change. We'd have to retain the ReportingMeasure type for backwards compatability and then what would be the point?