Facade_Engine: Added Facade GeneralMaterialTakeoff methods #3103
Conversation
There was a problem hiding this comment.
To the exception of the need for a fallback already mentioned by @FraserGreenroyd , the code looks sound.
As for the functionality itself, it would be good to have someone that understand the inputs and outputs better to review this PR. For me, I stared at the text Confirm Areas Exist for Layer Materials with a big arrow pointing to a NaN wondering if that was normal 😋 (for those the mass was not null so was that enough?).
|
@adecler I'll have @vgreen-BH review for functionality, but yea, the NaN for the first material and value for the second is as expected since the first material corresponds to the frame edge which is not an area based element. |
|
@BHoMBot check installer |
|
@enarhi to confirm, the following actions are now queued:
|
There was a problem hiding this comment.
On a code perspective, everything looks fine now, but will need functional review by @vgreen-BH before merging.
|
@BHoMBot check compliance |
|
@enarhi to confirm, the following actions are now queued:
|
|
@BHoMBot check required |
|
@enarhi to confirm, the following actions are now queued:
|
|
The check |
|
The check |
|
@BHoMBot check unit-tests |
|
@enarhi to confirm, the following actions are now queued:
|
|
FAO: @FraserGreenroyd The check they wish to have dispensation on is unit-tests. If you are providing dispensation on this occasion, please reply with:
|
|
@BHoMBot this is a DevOps instruction. I am authorising dispensation to be granted on check ref. 14998198547 |
|
@FraserGreenroyd I have now provided a passing check on reference |
|
@BHoMBot check ready-to-merge |
|
@FraserGreenroyd to confirm, the following actions are now queued:
|
Issues addressed by this PR
Closes #3091
Added
GeneralMaterialTakeoffmethods for Facade namespace, so that takeoffs for these elements properly account for area and volumes.Test files
Test File
To test, check the Panel, Opening, etc elements and confirm area takeoffs and LCA values work correctly for the Area EPD/material applied.
Additional comments
I have implemented this for Facade elements within Facade_Engine. Technically we could implement higher, but it seems to make sense with how @IsakNaslundBh has implemented this and others, allowing for specific approaches within namespaces as needed. It makes for a bit more redundant code, but allows for custom bits such as the implementation of opening area accounting for frame width required for Facade Openings.