SEED-platform / seed

Standard Energy Efficiency Data (SEED) Platform™ is a web-based application that helps organizations easily manage data on the energy performance of large groups of buildings.
Other
110 stars 55 forks source link

Meter data visible on some cycles but not all #1946

Closed RDmitchell closed 4 years ago

RDmitchell commented 5 years ago

Expected Behavior

Import a set of meter data for a record, then view it in the cycle that the data spans, ie, in this case the data is from 2018, and the user tried to view it through the Property Detail / Meter view coming from Cycle == 2018

Actual Behavior

The meter data does not display in the cycle == 2018, but in two other cycles, 2014 and 2016. There is no way to tell what cycle the meter data will be associated with. My understanding is that meter data is not associated with a cycle, but that seems like a problem given that users in general use cycles to organize their data, will have many years of data, and don't know which cycle the meter data will be visible from.

Steps to Reproduce

See the doc in this issue folder https://drive.google.com/open?id=1SfEBa4iLFkRzgLYdzMY695s25ANNrvrL

Instance Information

Instance: seeddemostaging SHA: e120119 (2.6.0 release)

RDmitchell commented 5 years ago

I think this is a problem that I didn't discover in testing because even though I had many cycles defined, and imported into all of them, none of them had the same data, ie, the same building list with the same ESPM ID numbers. In this case, there are several years of cycles, each with the same ESPM IDs in them. So maybe that is part of the why we are seeing this behavior. We will have to figure out how to deal with this, because as it is currently implemented, it's not really going to work for most users.

RDmitchell commented 5 years ago

I am also checking with the user to make sure that they didn't inadvertently set the cycle to 2016 when they imported the meter data -- I don't know if that is relevant or not.

RDmitchell commented 5 years ago

More info from the user.

More details in the doc referenced above.

RDmitchell commented 5 years ago

In a test org I made 4 cycles, 2015-2018, imported ESPM data with 2018 dates, then meter data (cycle set to 2015 on upload), and the data appeared in the Property Detail via the 2018 cycle. So with that simple set up, I couldn't replicate the behavior seen in the description above. I added this test info to the referenced doc.

adrian-lara commented 5 years ago

moved this to "In Progress" since this won't be ready to deploy until the linking PR is merged

RDmitchell commented 4 years ago

@adrian-lara -- I can't seem to import ESPM Meter data.

See the beginning of the doc for this issue for today's testing results

https://docs.google.com/document/d/1aNvRorrZDf746OTSrTyxJLoEVlCJCq_FVqP6hesvLP8/edit?usp=sharing

adrian-lara commented 4 years ago

@RDmitchell Was there an accompanying sentry error for the latest hang up given in the doc?

RDmitchell commented 4 years ago

@adrian-lara -- I didn't see a sentry error come through.

Are you able to import the ESPM meter data file for this issue in your local dev environment?

adrian-lara commented 4 years ago

Actually, I've got a recent copy of dev1, and I was able to reproduce that most recent error. This looks to be new and should likely be its own GH Issue. I'll create it since I've got the error message.

RDmitchell commented 4 years ago

@adrian-lara -- ok, thanks. Glad you can reproduce the problem.

adrian-lara commented 4 years ago

@nllong and @RDmitchell - I wanted to get my thoughts in writing, but of course, I'd be happy to discuss the following on a call.

There's a notable flaw in the way I built out the importing of PM meters. That is, the PM import feature for meters and meter readings was built on the assumption that PM Property ID would always be at least one of the matching fields. During a PM meter import, I assumed it would be enough to find just one record with a given PM Property ID and then associate meters to that lone record. The thought was that matching and linking would take care of associating the meters and meter readings to multiple records via that first found record.

Of course, as you both know, PM Property ID may or may not be considered "matching criteria" within any given organization. I think the ideal scenario is still to rely on PM Property ID during PM meter imports, but instead, apply incoming meters and meter readings to all records within a given cycle that have the given PM Property ID. I don't think this would be a massive effort, but I wanted to get both of your reactions on this before thinking about implementation steps.

Note that it would still be the case that meters are cycle-agnostic. Meaning, if a property has meters and meter readings and is linked to a property in another cycle, that other cycle will be associated to the same meters and meter readings.

RDmitchell commented 4 years ago

@nllong / @adrian-lara

I think it makes sense to hook the meters to properties by PM Property ID, even if that is not a matching field, and that the meter data would hook to all properties with that PM Property ID

RDmitchell commented 4 years ago

@adrian-lara -- should this get moved out of Dec 2019 Test and into March 2020 To do? Or something like that?

adrian-lara commented 4 years ago

@RDmitchell or @nllong

While importing Portfolio Manager meter readings, I'm able to associate these incoming meter readings to multiple records if they all have the given PM Property ID. Speaking on meters being cycle-agnostic, this currently includes records across all cycles of an organization. To help users understand what happens, I can update the results to provide the breakouts of cycles.

From a consistency standpoint, this makes sense to me, but I wanted to get your thoughts/reactions on a PM meter import applying meters on the appropriate records across all org cycles.

RDmitchell commented 4 years ago

@adrian-lara / @nllong -- I guess I am still a bit confused about this issue, ie, having the meters associated with cycles or not. I think it does make sense to disconnect them from cycles, since the meter data could be very different from the dates in a cycle.

So are you proposing that for the same record, looking at it in any cycle, the user would see the same meter data, ie, all the meter data that has ever been imported for that record (ie, PM Property ID)?

I think that makes sense. Not sure how the users will react, but maybe once we implement this first pass, we can ask them (maybe in our webinar we have with them before this release).

It might be useful to have some sort of "filter" (?) on the meter data to just show data that would be associated with the dates of a cycle (??). But this is just my first cut of ideas about that, and we definitely shouldn't implement anything like that without talking to users.

I think we should find out what the users "use cases" are for the meter data, after our first cut implementation, and that will inform what visual representations we can offer it to them in the future.

adrian-lara commented 4 years ago

It sounds like you've actually got a good handle on how it will work. Once linking is in, linked records will share the same set of meter readings.

Reading your comments, I feel a little more strongly that incoming meter imports should be cycle-agnostic in their search for records to be associated to.

Regarding this,

It might be useful to have some sort of "filter" (?) on the meter data

there is already filtering on the inventory detail meter page for users. Screen Shot 2020-03-03 at 3 52 24 PM

RDmitchell commented 4 years ago

@adrian-lara -- yes, this all seems good.

RDmitchell commented 4 years ago

instance: dev1 SHA: 64cc2d22

Worked as designed. I imported data into 4 different cycles, then imported ESPM Meter data into the last cycle, which was linked by PM Property ID to a record in all 4 cycles. Going to the detail view of that record in each cycle, then clicking on the Meter link, I could see the meter data in all the cycles.