Design conditions in OS:EpwFile #3086
Conversation
|
@jasondegraw FYI @joseph-robertson and @shorowit what are you planning to do with these methods? Simply avoid having to read the DDY file? Is data in the DDY file duplicated in the EPW file? |
|
Yes, we'd like to avoid having to read the DDY (so that our users don't have to supply it alongside the EPW). I don't know if all the data in the DYY is duplicated in the EPW header, but it has everything we need. |
|
And you will be making some method to take these values and create SizingPeriod:WeatherFileDays or SizingPeriod:WeatherFileConditionType objects with them? I have not used these objects very much, I don't know if they are even wrapped. Or is there enough info for you to make SizingPeriod:DesignDay objects directly? Should we add helper methods to the Model API or are you planning to just do this in a measure? |
|
No, we use the values in our own Manual J sizing calculations to hardsize coil objects, etc. We're not using E+ autosizing or sizing objects. |
joseph-robertson
changed the title
|
Is this done @joseph-robertson ? |
|
Yes @macumber. But I've only functionally tested using one epw file (vs all of them per your suggestion- ideas on how/where to do that?). |
There was a problem hiding this comment.
I am concerned about precision issues cropping up in some potential uses. If this object is used to directly create other objects, it could end up being a string to double to string situation. So we will need to be careful about that. With that said, the API looks pretty good, so it's good to merge.
macumber
added
the
Pull Request - Ready for CI
| // Could do a warning message here | ||
| return boost::none; | ||
| } | ||
| return boost::optional<std::string>(getUnits(id)); |
There was a problem hiding this comment.
You can just return getUnits(id).
There was a problem hiding this comment.
@macumber I'm confused why this wouldn't be an optional. Why does EpwDataPoint::getUnitsByName(...) return an optional then?
There was a problem hiding this comment.
The result is an optional, you just don't need to explicitly construct one like this. If you return a string, the compiler can implicitly convert that to an optional string.
| @@ -893,6 +1344,219 @@ namespace openstudio{ | |||
| return boost::none; | |||
| } | |||
|
|
|||
| boost::optional<double> EpwDesignCondition::getFieldByName(const std::string &name) | |||
There was a problem hiding this comment.
Should this be getFieldValueByName? getFieldByName seems like it would be returning EpwDesignField(name);
There was a problem hiding this comment.
Again, mimicking what EpwDataPoint does.
There was a problem hiding this comment.
Ok, to me getFieldByName would return a "field" which is basically an index. This is returning a "field value" so would be better as getFieldValueByName. If that is the naming convention in place we can stick with it, just trying to give you my feedback.
| /** Create an EpwDesignCondition from an EPW-formatted string */ | ||
| static boost::optional<EpwDesignCondition> fromDesignConditionsString(const std::string &line); | ||
| /** Create an EpwDesignCondition from a list of EPW designs as strings */ | ||
| static boost::optional<EpwDesignCondition> fromDesignConditionsStrings(const std::vector<std::string> &list); |
There was a problem hiding this comment.
Need a toDesignConditionsString() method?
| LOG(Error, "Expected at least 70 design condition fields rather than the " << split.size() << " fields in the EPW file '" << m_path << "'"); | ||
| return false; | ||
| } | ||
| else if (split.size() > 70) { |
There was a problem hiding this comment.
Shouldn't you check that nDesignConditions and split.size agree?
There was a problem hiding this comment.
Yes you should check them otherwise you'll get out of index errors below
There was a problem hiding this comment.
Will implement a check for making sure that nDesignConditions agrees with the expected length of the split array.
| return boost::none; | ||
| } | ||
|
|
||
| bool EpwFile::translateToWth(openstudio::path path, std::string description) |
| return false; | ||
| } | ||
|
|
||
| fp << "WeatherFile ContamW 2.0\n"; |
There was a problem hiding this comment.
Is this from @joseph-robertson or @jasondegraw ?
There was a problem hiding this comment.
That's from me... some years ago.
| openstudio::DateTime dateTime = data()[0].dateTime(); | ||
| openstudio::Time dt = timeStep(); | ||
| dateTime -= dt; | ||
| epwstrings[0] = std::to_string(dateTime.date().year()); |
There was a problem hiding this comment.
Need to check size before writing to index
No description provided.