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

Fix volume calculation to match EnergyPlus (Bugzilla #730) #127

Closed
axelstudios opened this issue Jul 19, 2013 · 14 comments · Fixed by #4592
Closed

Fix volume calculation to match EnergyPlus (Bugzilla #730) #127

axelstudios opened this issue Jul 19, 2013 · 14 comments · Fixed by #4592

Comments

@axelstudios
Copy link
Member

On 2012-06-08 14:19:09, @DavidGoldwasser wrote:

Just wanted to file this before I forgot about it. Current method will have problem with spaces that have sloped walls or ceilings.

On 2012-09-09 23:24:43, @DavidGoldwasser wrote:

I added BadSimResults but that would be an edge case where walls or roofs are not vertical.

On 2012-12-17 13:42:44, @DavidGoldwasser wrote:

This was inadvertently under C#

Keywords: BadSimResults

@ghost ghost assigned macumber Jul 19, 2013
@DavidGoldwasser
Copy link
Collaborator

@macumber I'll keep this open, but I know it is more of a feature than a bug, but it can be important for proper calculation of some internal load values on forward translation of complex geometry.

@macumber macumber changed the title Fix volume calculation to be more accurate (Bugzilla #730) Fix volume calculation to match EnergyPlus (Bugzilla #730) Mar 19, 2015
@agonzalezesteve
Copy link

I propose using the divergence theorem to compute the space volume, instead of multiplying the space area by a "mean" space height.
image
Then, using that every surface belongs to a plane, whose normal is parallel to its coefficients a, b and c;
image
only a constant integral is needed in every surface of the space.
image
Then, the volume integral reduces to
image
where d_i is the d coefficent of the plane where the i-th surface belongs and A_i is the area of the i-th surface.

I've tested it using this ruby code.
image

@eringold
Copy link
Contributor

@agonzalezesteve thank you, this is awesome.

I just tested the following single space geometry:
image

By hand, I calculate the volume at 17.6642 m^3 (measuring from sketchup in ft/in and converting ft^3 to m^3 in excel)
The default space.volume method for this space returns 19.0972 m^3
Your method returns 17.5825 m^3.
I'd say that's an improvement!

I've had many problems with OpenStudio geometry from sketchup ending up with EnergyPlus 'volume not enclosed' errors, to the point where I've taken to hard-assigning zone volume from a script. For complex spaces, I've had to hand-calc the volumes. I think this method is a huge improvement, and the dev team might consider implementing it to set zone volume at forward translation to avoid problems that EnergyPlus has calculating non-enclosed zone volumes with tiny surface gaps.

@macumber
Copy link
Contributor

The divergence calculation does not work for volumes which are not fully enclosed, I didn't implement it for OpenStudio because we don't have a check that the volumes are fully enclosed (no gaps or overlapping surfaces). I agree that it is best to match E+, I have been holding out for a solution where OS and E+ share code for calculations like this. That way we can make sure we are using the best available algorithms.

@eringold
Copy link
Contributor

@macumber ok maybe not implement it in OS by default, but the above algorithm is still better than zone volumes being set to 10 m^3 when E+ detects an edge that's not shared by two surfaces due to tiny gaps from sketchup. It will "work", just not "totally correctly" 😄

@macumber
Copy link
Contributor

I'm not really following @eringold, it sounds like you are having problems with the EnergyPlus calculation which fails due to tiny gaps (which invalidate the divergence calculation). Is that right? Is your problem with the OS calculation or the E+ one?

@eringold
Copy link
Contributor

eringold commented Apr 24, 2018

@macumber both. I've had problems with EnergyPlus failing to calculate volume from OS geometry that I just could not get to be fully enclosed when translated*, no matter how careful I was drawing it in OpenStudio. Hence the desire to hard-size zone volumes. But the easy way to do that is with the OpenStudio volumes, which can fail for funny-shaped zones like the one above (though I think the EnergyPlus volume calc might be more robust for that? - if it's fully enclosed). So I like the divergence method because, even though it's not perfect for unenclosed volumes (with, like, tiny gaps), the script above will still run and return a value close to the 'actual' volume, but can also handle complex geometry.

(I know it would be best to not have complex geometry in models to begin with, but stuff happens..)

*I meant to look into this in more detail but had a timeline that wouldn't allow for it.

@agonzalezesteve
Copy link

@macumber in which cases the geometry is not fully enclosed?

It is true that the divergence calculation doesn't work with unenclosed spaces, but the actual method neither if there is no floor or roof.

Furthermore, the actual method is a product of a "mean" height times the floor area, that in complex geometries this simplification fails.

In short, the current and the proposed methods for volume computation don't work with unenclosed volumes but the divergence method is more accurate in complex geometries.

@macumber
Copy link
Contributor

First of all, thank you @agonzalezesteve for this conversation, it is great to have you contributing here!

I don't know all the cases that geometry isn't fully enclosed. Thank you very much for your suggestion, I agree with your evaluation of the two approaches, the only thing I am unsure about is the robustness of both approaches. I was under the impression that you could get wildy wrong volumes from the divergence routine if you were missing a wall or something like that so the current routine is more robust to failure, I could be wrong here. In the end I suspect that a hybrid routine (e.g. use divergence if fully enclosed, issue warning and use some other method if not) would be best suited here. My hope was to implement this hybrid routine in a shared library used by both OpenStudio and EnergyPlus, I still think this is possible.

Currently OpenStudio does not hard set volume on translation to E+ so I don't think OpenStudio's current volume method affects most users. Am I missing some case where OpenStudio's volume calculation is commonly invoked? If a user wants to hard set volume in a measure then I think it is good for them to be able to choose between methods. I don't have a strong opinion on which one should be the default so my preference is to keep the current algorithm for now and point users to your snippet if they want to try that approach.

@agonzalezesteve
Copy link

@macumber thank you for the welcome!

You are right, the divergence method fails if one wall is missing and the current method is more robust in these cases.

I just tried to find a new volume algorithm because the current method gave me wrong values of air change per hour in spaces with complex geometry, like staircases inside buildings.

@jmarrec
Copy link
Collaborator

jmarrec commented Apr 8, 2021

double Space_Impl::volume() const {
double result = 0;
// TODO: need a better method
double roofHeight = 0;
int numRoof = 0;
double floorHeight = 0;
int numFloor = 0;
for (const Surface& surface : this->surfaces()) {
if (istringEqual(surface.surfaceType(), "Floor")) {
for (const Point3d& point : surface.vertices()) {
floorHeight += point.z();
++numFloor;
}
} else if (istringEqual(surface.surfaceType(), "RoofCeiling")) {
for (const Point3d& point : surface.vertices()) {
roofHeight += point.z();
++numRoof;
}
}
}
if ((numRoof > 0) && (numFloor > 0)) {
roofHeight /= numRoof;
floorHeight /= numFloor;
result = (roofHeight - floorHeight) * this->floorArea();
}
return result;
}

@jmarrec
Copy link
Collaborator

jmarrec commented Apr 8, 2021

The corresponding entry point for E+ calculation is around here: https://github.com/NREL/EnergyPlus/blob/29b5decc089394689a97932f66fa64d5a2fac062/src/EnergyPlus/SurfaceGeometry.cc#L11506-L11534

@wassimj
Copy link

wassimj commented Apr 4, 2022

This has not been fixed since 2013! I tried the workaround osThermalZone.setVolume but, while this fixes it for the .OSM file, it does not fix it for the .gbxml output which continues to calculate the wrong volume for more complex shapes (mainly slanted hip roofs etc).

@jmarrec
Copy link
Collaborator

jmarrec commented May 23, 2022

I have implemented a fix for this long overdue issue in #4592, would appreciate a review from anyone interested

This was referenced May 23, 2022
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.

7 participants