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

HoursPerDay PoolPump element intended to be a child of PumpSpeed? #11

Closed
andybardwell opened this issue Nov 21, 2014 · 13 comments
Closed

Comments

@andybardwell
Copy link

Was the HoursPerDay PoolPump element intended to be a child of PumpSpeed? Pump calculations use time at different speeds. The current configuration seems to support only single speed pumps sinc it only allows time-of-use for one speed. What is the intent of the single SpeedSetting (low, high, most efficient, other, unknown, none) and and HoursPerDay for PoolPump, versus the multiple PumpSpeed elements without a time of use? The HoursPerDay annotation is "Number of hours per day a pool pump operates at a particular speed setting."

@nmerket nmerket added the bug label Dec 10, 2014
@nmerket nmerket added this to the v2.1 milestone Dec 10, 2014
@nmerket
Copy link
Contributor

nmerket commented Dec 10, 2014

For reference, here's the diagram of what Andy's talking about.

baseelements_poolpump

@nmerket
Copy link
Contributor

nmerket commented Dec 10, 2014

@andybardwell This change makes sense to me with my very limited knowledge of pool pumps. I have questions about how this information could possibly be gathered in an audit. I'm hopeful someone with more domain knowledge will have something to say.

However, most importantly, I just want to note that this will be a breaking change, which has implications usually requiring this to be done on a major release.

@kmwoley
Copy link

kmwoley commented Dec 10, 2014

EnergySavvy is okay with this particular breaking change since we are not
actively using PoolPumps in our HPXML.

On Wed, Dec 10, 2014, 11:17 AM Noel Merket notifications@github.com wrote:

@andybardwell https://github.com/andybardwell This change makes sense
to me with my very limited knowledge of pool pumps. I have questions about
how this information could possibly be gathered in an audit. I'm hopeful
someone with more domain knowledge will have something to say.

However, most importantly, I just want to note that this will be a
breaking change, which has implications usually requiring this to be done
on a major release.


Reply to this email directly or view it on GitHub
#11 (comment).

@andrulis
Copy link
Contributor

I agree that this change makes sense. PSD is just now addressing supporting Pool Pumps, so we are also okay with this breaking change.

@nmerket
Copy link
Contributor

nmerket commented Dec 15, 2014

Does anybody remember who in California designed this? I'd like to drag them into the conversation.

@juliecaracino?

@juliecaracino
Copy link
Contributor

Yes, I worked with Jeff Farlow (jeff.farlow@pentair.com) and others from PG&E to develop this sub-tree. I emailed Jeff last week to comment. He said he was speaking with Andy about this.

@andybardwell
Copy link
Author

Jeff and I reviewed the HTML HPXML tree view, and he agreed with the
proposed change.


OptiMiser LLC
Andy Bardwell, Ph.D., CEO
Cell: 720-219-3627
Fax: 888-228-4751
andy@optimiserenergy.com
www.OptiMiserEnergy.com http://www.optimiserenergy.com/
Introductory Video Online https://vimeo.com/49039973

On Mon, Dec 15, 2014 at 8:49 AM, juliecaracino notifications@github.com
wrote:

Yes, I worked with Jeff Farlow (jeff.farlow@pentair.com) and others from
PG&E to develop this sub-tree. I emailed Jeff last week to comment. He said
he was speaking with Andy about this.


Reply to this email directly or view it on GitHub
#11 (comment).

@nmerket
Copy link
Contributor

nmerket commented Dec 16, 2014

So, my understanding of this is that we'd like to move the HoursPerDay element to be under PumpSpeed. Is that correct? Is there any other change being requested here?

Also, one idea I had to address the backwards compatibility issue would be to add an HoursPerDay element to PumpSpeed while also leaving it under PoolPump. I could then mark in the annotation that it has been deprecated in favor of the newer one. Then when we release 3.0, we can remove it. Does that sound like a reasonable solution or just potentially problematic because we're leaving that extra element hanging around asking to be used when it probably shouldn't.

@andybardwell
Copy link
Author

That is correct. Your suggestion of adding an element and removing the
deprecated one later sounds clever and should avoid any consternation.


OptiMiser LLC
Andy Bardwell, Ph.D., CEO
Cell: 720-219-3627
Fax: 888-228-4751
andy@optimiserenergy.com
www.OptiMiserEnergy.com http://www.optimiserenergy.com/
Introductory Video Online https://vimeo.com/49039973

On Tue, Dec 16, 2014 at 3:40 PM, Noel Merket notifications@github.com
wrote:

So, my understanding of this is that we'd like to move the HoursPerDay
element to be under PumpSpeed. Is that correct? Is there any other change
being requested here?

Also, one idea I had to address the backwards compatibility issue would be
to add an HoursPerDay element to PumpSpeed while also leaving it under
PoolPump. I could then mark in the annotation that it has been deprecated
in favor of the newer one. Then when we release 3.0, we can remove it. Does
that sound like a reasonable solution or just potentially problematic
because we're leaving that extra element hanging around asking to be used
when it probably shouldn't.


Reply to this email directly or view it on GitHub
#11 (comment).

@nmerket
Copy link
Contributor

nmerket commented Dec 16, 2014

This is what it looks like now:

baseelements_poolpump

@nmerket
Copy link
Contributor

nmerket commented Dec 16, 2014

If nobody has any objections, I'll close this tomorrow.

@andybardwell
Copy link
Author

Thanks, Noel!


OptiMiser LLC
Andy Bardwell, Ph.D., CEO
Cell: 720-219-3627
Fax: 888-228-4751
andy@optimiserenergy.com
www.OptiMiserEnergy.com http://www.optimiserenergy.com/
Introductory Video Online https://vimeo.com/49039973

On Tue, Dec 16, 2014 at 4:35 PM, Noel Merket notifications@github.com
wrote:

If nobody has any objections, I'll close this tomorrow.


Reply to this email directly or view it on GitHub
#11 (comment).

@jefffarlow
Copy link

I too agree with the change, Thanks.
Please feel free to reach out to me directly regarding any pool related issues.
(jeff.farlow@pentair.com)

@nmerket nmerket closed this as completed Dec 17, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants