Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add weather station element #267

Merged
merged 5 commits into from
Dec 16, 2020
Merged

Add weather station element #267

merged 5 commits into from
Dec 16, 2020

Conversation

macintoshpie
Copy link
Contributor

No description provided.

Comment on lines +42 to +47
<xs:complexType name="TimeSeriesType">
<xs:sequence>
<!-- ... other sub elements ... -->
<xs:element name="WeatherStationID" minOccurs="0">
<xs:annotation>
<xs:documentation>ID number of weather station this time series contributes to.</xs:documentation>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would like some feedback on "WeatherStationID" as reference. This might be confusing to users, e.g. the might think of the NOAA station ID here. Though this naming is consistent with our other ID reference elements. Is there something we should add to the documentation to make this more clear?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without this proposal, is it possible to link a time series to a non-auc:WeatherStations weather station of a specific site or building (i.e., specified by auc:WeatherDataStationID, etc.)?

With this proposal, is it possible to link a time series to a non-auc:WeatherStations weather station of a specific site or building?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(1) -- No, it's not possible currently. You can provide weather station information inside of the scenario (see auc:WeatherType, but there's no explicit link)

(2) -- No, you are not able to link to non-auc:WeatherStations in this proposal

So it's not really a concern about creating a valid doc, its just a concern about how it might be interpreted by a user

Copy link
Contributor

@corymosiman12 corymosiman12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I have a definitive answer yet. One thing that jumps out to me is that this is a slightly different design than how we currently make references to auc:ResourceUse elements. For the auc:ResourceUse, we don't currently declare the property that is being measured within the auc:ResourceUse itself (or within the auc:TimeSeries element either). The property (energy, power, etc.) is assumed, i.e.

<auc:ResourceUse ID="Electricity">
  <auc:EnergyResource>Electricity</auc:EnergyResource>
 ...
</auc:ResourceUse>
...
<auc:TimeSeries ID="TS1">
    <auc:ReadingType>Total</auc:ReadingType>
    <!-- We assume 'Total' means energy, although we don't explicitly declare that -->
    <auc:StartTimestamp>2020-02-01T00:00:00+00:00</auc:StartTimestamp>
    <auc:EndTimestamp>2020-03-01T00:00:00+00:00</auc:EndTimestamp>
    <auc:IntervalReading>70</auc:IntervalReading>
    <auc:ResourceUseID IDref="Electricity"/>
</auc:TimeSeries>
<auc:TimeSeries ID="TS2">
    <auc:ReadingType>Peak</auc:ReadingType>
    <!-- We assume 'Peak' means power, although we don't explicitly declare that -->
    <auc:StartTimestamp>2020-02-01T00:00:00+00:00</auc:StartTimestamp>
    <auc:EndTimestamp>2020-03-01T00:00:00+00:00</auc:EndTimestamp>
    <auc:IntervalReading>70</auc:IntervalReading>
    <auc:ResourceUseID IDref="Electricity"/>
</auc:TimeSeries>

@macintoshpie, in your proposal, we aren't actually declaring the substance being measured at the weather station (like we do with the auc:ResourceUse), we are simply assuming the substance is 'Air'. This could be fine, but weather stations will also be measuring things like irradiance (sunlight?), windspeed, etc.

That said, I'm gonna make this suggestion to give us another design option:

<auc:WeatherStation>
    <auc:Substances> <!-- Similar to ResourceUses -->
        <auc:Substance ID="Air"> <!-- Similar to ResourceUse -->
            <auc:SubstanceType>Air</auc:SubstanceType>
        </auc:Substance>
    </auc:Substance>
</auc:WeatherStation>
...
<auc:TimeSeries ID="TS2">
    <auc:ReadingType>Average</auc:ReadingType>
    <auc:TimeSeriesReadingQuantity>Dry Bulb Temperature</auc:TimeSeriesReadingQuantity>
    <auc:StartTimestamp>2020-02-01T00:00:00+00:00</auc:StartTimestamp>
    <auc:EndTimestamp>2020-03-01T00:00:00+00:00</auc:EndTimestamp>
    <auc:IntervalReading>70</auc:IntervalReading>
    <auc:SubstanceID IDref="Air"/>
</auc:TimeSeries>

And say that for consistencies sake, we also start requiring a auc:TimeSeriesReadingQuantity be declared for timeseries relating to resources as well:

<auc:TimeSeries ID="TS1">
    <auc:ReadingType>Total</auc:ReadingType>
    <auc:TimeSeriesReadingQuantity>Energy</auc:TimeSeriesReadingQuantity> <!-- Explicit -->
    <auc:StartTimestamp>2020-02-01T00:00:00+00:00</auc:StartTimestamp>
    <auc:EndTimestamp>2020-03-01T00:00:00+00:00</auc:EndTimestamp>
    <auc:IntervalReading>70</auc:IntervalReading>
    <auc:ResourceUseID IDref="Electricity"/>
</auc:TimeSeries>
<auc:TimeSeries ID="TS2">
    <auc:ReadingType>Peak</auc:ReadingType>
    <auc:TimeSeriesReadingQuantity>Power</auc:TimeSeriesReadingQuantity> <!-- Explicit -->
    <auc:StartTimestamp>2020-02-01T00:00:00+00:00</auc:StartTimestamp>
    <auc:EndTimestamp>2020-03-01T00:00:00+00:00</auc:EndTimestamp>
    <auc:IntervalReading>70</auc:IntervalReading>
    <auc:ResourceUseID IDref="Electricity"/>
</auc:TimeSeries>

@macintoshpie
Copy link
Contributor Author

macintoshpie commented Nov 18, 2020

@corymosiman12 thanks for the feedback

we aren't actually declaring the substance being measured at the weather station (like we do with the auc:ResourceUse), we are simply assuming the substance is 'Air'. This could be fine, but weather stations will also be measuring things like irradiance (sunlight?), windspeed, etc.

I think the auc:Substances is interesting, but if we're adding it solely to clarify what we're measuring, then I think we should just add that to the auc:TimeSeriesReadingQuantity enum. Also, from my understanding Dry bulb temperature by definition is the temperature of air ref, so we don't need to be any more explicit that we are recording air temperature.

If we were to add other weather record types (irradiance, windspeed) wouldn't they be understandable as their own auc:TimeSeriesReadingQuantity enums? In fact we already have these as options:

            <xs:enumeration value="Humidity ratio"/>
            <xs:enumeration value="Relative humidity"/>
            <xs:enumeration value="Diffuse Horizontal Radiation"/>
            <xs:enumeration value="Direct Normal Radiation"/>
            <xs:enumeration value="Global Horizontal Radiation"/>
            <xs:enumeration value="Dry Bulb Temperature"/>
            <xs:enumeration value="Wet Bulb Temperature"/>
            <xs:enumeration value="Wind Speed"/>

On the otherhand, if we intend to add other things to an auc:Substance, then I think it's worth breaking out. E.g. maybe there's some aggregate values we want to include for an auc:Substance like AnnualAverage... like we have for auc:ResourceUses. In that case I could understand creating "substances" for a weather station.

Also, if we go down the substance route, should it be something like auc:Sensor instead?

@corymosiman12
Copy link
Contributor

@macintoshpie I think probably for a 2.X stuff, we should stick with your proposal, it makes sense. Regarding IDs, maybe we can use something similar to auc:PremisesIdentifier, give people a set of enums for weather station ID values, and then a label. Then we can separate the ID of the element from multiple IDs that may be used. Something like:

<auc:WeatherStationIdentifiers>
  <auc:WeatherStationIdentifier>
    <auc:Label>NOAA ID</auc:Label>
    <auc:Value>abc.124</auc:Value>
  </auc:WeatherStationIdentifier>
</auc:WeatherStationIdentifiers>

Copy link
Member

@nllong nllong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This proposal looks great, thanks!

@corymosiman12
Copy link
Contributor

@markborkum - thoughts?

@corymosiman12 corymosiman12 self-requested a review December 8, 2020 04:07
Copy link
Contributor

@markborkum markborkum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Proposal looks good, thanks.

NOTE: this begins the deprecation of the original, unnested weather station data
Copy link
Member

@nllong nllong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice, thanks!

@nllong nllong merged commit 4db7102 into develop Dec 16, 2020
@nllong nllong deleted the proposal/weather-data branch December 16, 2020 17:19
@nllong nllong added the Schema: General General update to BuildingSync label Dec 17, 2020
@nllong nllong changed the title proposal: add proposal for weather station entity Add weather station element Dec 17, 2020
@nllong nllong added enhancement Non-breaking Change feature Adding new functionality to BuildingSync and removed enhancement labels Dec 17, 2020
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
feature Adding new functionality to BuildingSync Non-breaking Change Schema: General General update to BuildingSync
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants