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

Allow multiple Attic Floor Insulation and Attic Roof Insulation nodes #28

Closed
andrulis opened this issue Mar 30, 2015 · 22 comments
Closed

Comments

@andrulis
Copy link
Contributor

When modeling an attic, there may be multiple insulation sections for the floor or roof yet really be part of the same attic node. Currently, if there are multiple insulations sections, we need to artificially create multiple attics. Instead, I propose we can allow multiple attic floor insulation nodes and multiple attic roof insulation nodes.

@nmerket
Copy link
Contributor

nmerket commented Apr 7, 2015

Oh boy, this takes something that's already kind of complicated and makes in complicateder. I see your reasoning and it makes sense. It would also be "non-breaking" in the strict sense that old files would validate against the updated schema. However, the extra layer of multiplicity it adds would be a real bear for receiving systems (one of which I maintain now), so I'm hesitant to jump on this one without a wider discussion and consensus.

@GamalielL
Copy link

Because this is backwards compatible, it wouldn't cause any problems for
us. However, I'm of the general opinion that we shouldn't be adding
complications like this unless someone on the program implementation side
is pushing hard for them.

On Tue, Apr 7, 2015 at 3:36 PM, Noel Merket [email protected]
wrote:

Oh boy, this take something that's already kind of complicated and makes
in complicateder. I see your reasoning and it makes sense. It would also be
"non-breaking" in the strict sense that old files would validate against
the updated schema. However, the extra layer of multiplicity it adds would
be a real bear for receiving systems (one of which
https://github.com/NREL/hescore-hpxml I maintain now), so I'm hesitant
to jump on this one without a wider discussion and consensus.


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

@kmwoley
Copy link

kmwoley commented Apr 7, 2015

Agreed.

On Tue, Apr 7, 2015 at 12:44 PM GamalielL [email protected] wrote:

Because this is backwards compatible, it wouldn't cause any problems for
us. However, I'm of the general opinion that we shouldn't be adding
complications like this unless someone on the program implementation side
is pushing hard for them.

On Tue, Apr 7, 2015 at 3:36 PM, Noel Merket [email protected]
wrote:

Oh boy, this take something that's already kind of complicated and makes
in complicateder. I see your reasoning and it makes sense. It would also
be
"non-breaking" in the strict sense that old files would validate against
the updated schema. However, the extra layer of multiplicity it adds
would
be a real bear for receiving systems (one of which
https://github.com/NREL/hescore-hpxml I maintain now), so I'm hesitant
to jump on this one without a wider discussion and consensus.


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


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

@nmerket nmerket added this to the v2.2 milestone Oct 19, 2015
@nmerket
Copy link
Contributor

nmerket commented Oct 19, 2015

I'm adding the 2.2 milestone because we should decide whether we're doing this or not doing this for that release.

@nmerket nmerket added schemas and removed schemas labels Nov 30, 2015
@nmerket
Copy link
Contributor

nmerket commented Nov 30, 2015

Right now I have the following tally then:

for against
@andrulis @kmwoley
@GamalielL
@nmerket

Reasons to add

  • Allows specification of real attics where there are areas of the attic floor and/or roof deck having different insulation levels.
  • Otherwise more than one attic must be defined to describe the physical properties of one.

Reasons not to add

  • Forces those interpreting HPXML to have one more layer of multiplicity to deal with.
  • The area is already on the Attic element. Would need to be moved down to the insulation elements, breaking backwards compatibility.

Conclusion

I'm leaning towards not doing this. @andrulis if you can persuade us otherwise, now is your chance.

@andrulis
Copy link
Contributor Author

Right now to model an attic in a way that Savvy can interpret it for NYSERDA, we need to artificially create multiple attics (one attic for every surface in the attic) and add an artificial area of 1 (since the Area node has a min exclusive of 0) when we are referring to a roof section. There is no way to tell just how many attic spaces are actually in the home and the receiving systems needs to know to ignore if an attic Area = 1.

@jjaugenbraun
Copy link

As I understand it, what we are doing at NYSERDA is actually the original intent of how to use the attic nodes in HPXML (with the exception of the Area = 1 workaround that is needed due to one of the savings reasonableness rules at NYSERDA). Since this process works for all of the modeling tools or they have a functioning workaround in place, I vote that we maintain the status quo.

@GamalielL
Copy link

@andrulis can you explain why you end up needing to create Area=1 attics?
That's not something we have run into.

On Mon, Nov 30, 2015 at 6:10 PM, jjaugenbraun [email protected]
wrote:

As I understand it, what we are doing at NYSERDA is actually the original
intent of how to use the attic nodes in HPXML (with the exception of the
Area = 1 workaround that is needed due to one of the savings reasonableness
rules at NYSERDA). Since this process works for all of the modeling tools
or they have a functioning workaround in place, I vote that we maintain the
status quo.


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

@jjaugenbraun
Copy link

Hi Gamaliel,

Given the way NYSERDA attic insulation measures are defined, if the actual attic has improvements to both the AtticFloorInsulation and the AtticRoofInsulation systems, that would be conveyed as two separate attics in HPXML and two separate measures. Since each of those attics requires an area to be specified according to the NYSERDA data set and the HPXML schema does not permit an area of 0, we are using 1 so we can easily ignore these areas for reporting purposes and avoid double counting attic area (since both the AtticFloorInsulation and AtticRoofInsulation would be in the same actual attic in this case).

Thanks,
JJ

@GamalielL
Copy link

JJ - That makes sense from an HPXML perspective, but I still don't
understand the real-life situation where you would be improving the
insulation in both surfaces in one attic. It's my understanding that codes
in NY require you to remove attic floor insulation, if you are going to
insulate the roof deck.

On Mon, Nov 30, 2015 at 10:43 PM, jjaugenbraun [email protected]
wrote:

Hi Gamaliel,

Given the way NYSERDA attic insulation measures are defined, if the actual
attic has improvements to both the AtticFloorInsulation and the
AtticRoofInsulation systems, that would be conveyed as two separate attics
in HPXML and two separate measures. Since each of those attics requires an
area to be specified according to the NYSERDA data set and the HPXML schema
does not permit an area of 0, we are using 1 so we can easily ignore these
areas for reporting purposes and avoid double counting attic area (since
both the AtticFloorInsulation and AtticRoofInsulation would be in the same
actual attic in this case).

Thanks,
JJ


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

@nmerket
Copy link
Contributor

nmerket commented Dec 1, 2015

Having to do a workaround hack like that isn't desirable. If making this change makes it so you don't have to do that, I'm all for it. I guess the question I have is whether allowing multiple AtticFloorInsulation and AtticRoofInsulation nodes would actually fix it since the Area is a subelement of the Attic and not the insulation nodes.

hpxml_attic

If I could do this all over again I'd put the roof insulation on the Roof node. Maybe v3.

@jjaugenbraun
Copy link

@GamalielL I'm not sure how often this happens in real life. @andrulis Do you have a sense for that?

@nmerket Since we have a workaround in place and this is a breaking change that not everyone may have had a chance to review, do you think we could table this change until v2.3?

@andrulis
Copy link
Contributor Author

andrulis commented Dec 2, 2015

The real life scenario is that only part of the attic floor is insulated. Now, I have to create separate attic nodes for each variation in attic floor or roof insulation. So instead of the attic indicating the concept of the attic 'space' and the floor and roof insulation indicating the concept of the 'surfaces' of that 'space', we have to create a separate 'space' for each 'surface' and suddenly I have no idea if this building had one attic or multiple attics.

I do like the idea of moving the roof insulation to the Roof node. Perhaps we can do that in this release and deprecate the current attic roof insulation node with the idea to remove it in 3.0

@GamalielL
Copy link

The problem is that Attics are halfway between a standalone insulation
surface component (like Wall) and a defined space (like Foundation) with an
unbounded number of insulation surface sub-nodes that each have their own
area. I think that Rich would like them to be more like Foundations. One
problem with that is that there can be insulated roof components that don't
belong to an Attic. We already have AttachedToRoof/idref and
AtticKneeWall#/idref element in the Attic. I think that the ideal scenario
would be to take that a step further and specify all insulation surfaces
outside of the Attic node, as follows:

  • Add a RoofInsulation insulation sub-node to the Roof node
  • Create a new AtticFloor node with Area element and
    AtticFloorInsulation sub-node
  • Add AttachedToFloor/idref and AttachedToWall/idref elments to the
    Attic node, so the Attic space can be associated with all bounding surfaces
    (roofs, gable walls, knee walls, and attic floors)

If we leave the existing insulation nodes inside Attic, then we can
maintain backwards compatibility for the time being. They could be removed
in V3.

On Wed, Dec 2, 2015 at 12:52 PM, andrulis [email protected] wrote:

The real life scenario is that only part of the attic floor is insulated.
Now, I have to create separate attic nodes for each variation in attic
floor or roof insulation. So instead of the attic indicating the concept of
the attic 'space' and the floor and roof insulation indicating the concept
of the 'surfaces' of that 'space', we have to create a separate 'space' for
each 'surface' and suddenly I have no idea if this building had one attic
or multiple attics.

I do like the idea of moving the roof insulation to the Roof node. Perhaps
we can do that in this release and deprecate the current attic roof
insulation node with the idea to remove it in 3.0


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

@nmerket
Copy link
Contributor

nmerket commented Dec 3, 2015

I will add an area to the insulation elements and post a pull request. Stay posted.

@GamalielL
Copy link

Noel - Please review my proposal first.

On Thu, Dec 3, 2015 at 2:52 PM, Noel Merket [email protected]
wrote:

I will add an area to the insulation elements and post a pull request.
Stay posted.


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

@nmerket
Copy link
Contributor

nmerket commented Dec 3, 2015

@GamalielL, will do.

@nmerket
Copy link
Contributor

nmerket commented Dec 17, 2015

@GamalielL I think I like what you're proposing here, but having both in there for v2.2 is going to be a compatibility nightmare for receiving systems. Even though it's technically backwards compatible, it breaks the spirit of backwards compatibility. It will make anyone receiving HPXML have to parse both places and interpret it differently based on which elements are used (old or new). What if both are specified?

A general re-working of attics is a great idea and it should be done. In v3.0.

@GamalielL
Copy link

Let's leave this alone for now then. I don't think it is adversely
affecting any programs at this point.

On Thu, Dec 17, 2015 at 2:15 PM, Noel Merket [email protected]
wrote:

@GamalielL https://github.com/GamalielL I think I like what you're
proposing here, but having both in there for v2.2 is going to be a
compatibility nightmare for receiving systems. Even though it's technically
backwards compatible, it breaks the spirit of backwards compatibility. It
will make anyone receiving HPXML have to parse both places and interpret it
differently based on which elements are used (old or new). What if both are
specified?

A general re-working of attics is a great idea and it should be done. In
v3.0.


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

@andrulis
Copy link
Contributor Author

If you guys had seen the rather crappy workaround we had to do to make this work with the NYSERDA program, I think you'd be more agreeable to making the change now. I really liked Gamaliel's proposal and I would argue that it is better to add it now and deprecate the existing functionality in v3. That is a normal software versioning process. Add a replacement feature in one release and deprecate the old feature in the next. This gives software vendors time to phase in the change rather than having to scramble to adjust once v3 hits.

@nmerket
Copy link
Contributor

nmerket commented Feb 1, 2016

Still not quite sure what to do here, guys. Let's talk it through on the call Thursday.

@nmerket nmerket modified the milestones: v3.0, v2.2 Feb 4, 2016
@shorowit
Copy link
Contributor

shorowit commented Feb 7, 2019

This is addressed by the proposed attic refactor for v3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants