Open aothms opened 2 years ago
Thank you @aothms! The new properties are in three batches (Road, P&W and Rail). We need to fix those. We are not mandated to fix the old ones though. As a matter of fact we were told not to touch them. Road and P&W were not properly QAed and many were added manually to the spec. For Rail we have a review process and some automatic checks and type assignments, so these should be much better.
Regarding template types, I am not sure that that is an error since the attribute is optional. @TLiebich @MatthiasWeise IfcPropertySetTemplate.TemplateType
is optional, should we always set it anyway?
On some of the issues:
to 1. Not sure what to do with this one as IfcCivilElement
is deprecated. Probably deprecate the Pset also?
to 2. -5. Old issues.
to 6. @larswik our responsibility
to 7. -10. old
to 38. This is interesting. It is a valid type and it is also assigned to both IfcTransportElement
and IfcTransportElementType
. @AlexBrad1ey can give us the best explanation and background.
- Should MaterialColour be a reference property to a IfcColourRGB instead?
I do not see IfcColourRGB
in the IfcObjectReferenceSelect
. Would we want to add this? Perhaps this is the reason that the PrimaryDataType
was not assigned. @larswik would know more.
On the TemplateTypes I would like to hear from @TLiebich and @MatthiasWeise.
And the last point I would as @AlexBrad1ey @AlexBrad1eyCT (just referencing both, not sure which one is active 😄 ).
I would also use this issue to point out a couple more. Forgot a couple of times now to post these:
IfcLinearVelocityMeasure
IfcPressureMeasure
Thanks for the quick and detailed reply @SergejMuhic :)
Let's await responses from the others.
Good point about the IfcObjectReferenceSelect. I hadn't thought of that. Probably anyway reference properties aren't that well supported in software, so maybe it's better to stick to simple/enum value.
Quickly going over them. Current properties re colour are not very consistent, they are either IfcText IfcLabel or PEnum_CoreColorsEnum (BLACK BLUE BROWN GOLD GREEN GREENANDYELLOW GREY NOTKNOWN ORANGE OTHER PINK RED SILVER TURQUOISE UNSET VIOLET WHITE YELLOW) and a specific one for sprinklers.
Not sure what is wise here. PropertyListValue<IfcNormalizedRatioMeasure> is an option, but it seems like users probably want to (also..) be able to specify named colors. The current enum is a bit odd (no purple?), https://en.wikipedia.org/wiki/Web_colors#HTML_color_names has some ideas.
cc @berlotti because a consistent representation of colours touches upon prop normalization
I will look in to these this week, thanks for picking this up detailed response to follow.
To be completed after Property Set Work Finishes
When going over the RC4_Update psets I found the following issues:
So 3 categories of issues:
Missing underlying type for TypePropertySingleValue
Most of these I can figure out myself in the migration. LoadBearing is obviously a bool. A bit surpised perhaps to see Pset_(Stair|Ramp)Common.LoadBearing and Pset_RampCommon.ThermalTransmittancebut but those are not 4x3 issues I suppose.
Should MaterialColour be a reference property to a IfcColourRGB instead?
Missing template type (PSET_OCCURRENCEDRIVEN, PSET_PERFORMANCEDRIVEN, PSET_TYPEDRIVENOVERRIDE)
If we can agree on some default (maybe there already is, I don't know) then it's not an issue I assume, unless you feel like these psets should deviate from that default
Pset_MarineVehicleCommon has a template type of QTO_TYPEDRIVENOVERRIDE, I doubt that can be correct, but perhaps it is?