search

buildingSMART / IFC4.4.x-development

Development of IFC 4.4
Other
8 stars 6 forks source link

Inconsistent comma separated values in pset template definitions #10

Open Moult opened 3 years ago

Moult commented 3 years ago

Some pset template definitions use comma separated values with spaces for their applicable entities, like so:

Pset_SpaceFireSafetyRequirements --> 'IfcSpace, IfcSpatialZone, IfcZone'

Others do not:

Pset_ConcreteElementGeneral --> 'IfcBeam,IfcBuildingElementProxy,IfcChimney,IfcColumn,IfcFooting,IfcMember,IfcPile,IfcPlate,IfcRailing,IfcRamp,IfcRampFlight,IfcRoof,IfcSlab,IfcStair,IfcStairFlight,IfcWall,IfcCivilElement'

The example given in the documentation does not use spaces.

I propose to remove all spaces in comma separated lists. Ping @berlotti @TLiebich @aothms

aothms commented 3 years ago

I'd say this needs to be properly encoded in the schema. It can be an express list, like:

PSETTEMPLATE(..., (ENTITY('IfcSpace'), ENTITY_PREDEFINEDTYPE('IfcSpace', 'PARKING')));

Moult commented 3 years ago

@aothms I think there is a misunderstanding here - this is referring to the ApplicableEntity attribute of IfcPropertySetTemplate which has a data type of IfcIdentifier. How do you propose to enforce an encoding in the IfcIdentifier field?

TLiebich commented 3 years ago

semantically correct would be

ApplicableEntities : OPTIONAL LIST [1:?] OF IfcIdentifier;

however this would introduce a change, that we have identified as non-upward compatible in the past, since it changes the exchange structure significantly

now buildingSMART/IFC4.3.x-development#1=IFCPROPERTYSETTEMPLATE ( .... , 'IfcWall, IfcBeam', ...); then buildingSMART/IFC4.3.x-development#1=IFCPROPERTYSETTEMPLATE ( .... , ('IfcWall', 'IfcBeam'), ...);

so my proposal for now is to stay with a single STRING and enforce a particular syntax, and leave a more general change to IFC5.

Moult commented 3 years ago

Yes, I think that is a great solution! As you say, it is non upward compatible, so how about for now we enforce no spaces, and fix any occurrences that have spaces? Here is the revised proposal:

Solution for IFC4.3: Enforce no spaces. Fix any occurring spaces.

Solution for IFC5: ApplicableEntities : OPTIONAL LIST [1:?] OF IfcIdentifier; now buildingSMART/IFC4.3.x-development#1=IFCPROPERTYSETTEMPLATE ( .... , 'IfcWall, IfcBeam', ...); then buildingSMART/IFC4.3.x-development#1=IFCPROPERTYSETTEMPLATE ( .... , ('IfcWall', 'IfcBeam'), ...);

Moult commented 2 years ago

Marked as onhold due to backwards compatibility issues.

aothms commented 2 years ago

This is already solved in the latest code gen right, which is be definition consistent?

Sent from a mobile device. Excuse my brevity. Kind regards, Thomas

Op wo 2 mrt. 2022 11:12 schreef Dion Moult @.***>:

Some pset template definitions use comma separated values with spaces for their applicable entities, like so:

Pset_SpaceFireSafetyRequirements --> 'IfcSpace, IfcSpatialZone, IfcZone'

Others do not:

Pset_ConcreteElementGeneral --> 'IfcBeam,IfcBuildingElementProxy,IfcChimney,IfcColumn,IfcFooting,IfcMember,IfcPile,IfcPlate,IfcRailing,IfcRamp,IfcRampFlight,IfcRoof,IfcSlab,IfcStair,IfcStairFlight,IfcWall,IfcCivilElement'

The example given in the documentation does not use spaces.

I propose to remove all spaces in comma separated lists. Ping @berlotti https://github.com/berlotti @TLiebich https://github.com/TLiebich @aothms https://github.com/aothms

— Reply to this email directly, view it on GitHub https://github.com/buildingSMART/IFC4.4.x-development/issues/10, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAILWV63SOHVPR62LJOLN2DU545IDANCNFSM5PW7ANAA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you were mentioned.Message ID: @.***>

Moult commented 2 years ago

@aothms well actually this only appears in IFC pset templates, and to my knowledge I only see .xml outputs not .ifc so this hasn't yet been solved?

aothms commented 2 years ago

ApplicableType="IfcSpace, IfcSpatialZone, IfcZone"

https://github.com/bSI-InfraRoom/IFC-Specification/blob/develop/IFC4x3/Sections/Core%20data%20schemas/Schemas/IfcProductExtension/PropertySets/Pset_SpaceFireSafetyRequirements/DocPropertySet.xml

ApplicableType="IfcBeam,IfcBuildingElementProxy,IfcChimney,IfcColumn,IfcFooting,IfcMember,IfcPile,IfcPlate,IfcRailing,IfcRamp,IfcRampFlight,IfcRoof,IfcSlab,IfcStair,IfcStairFlight,IfcWall,IfcCivilElement"

https://github.com/bSI-InfraRoom/IFC-Specification/blob/develop/IFC4x3/Sections/Domain%20specific%20data%20schemas/Schemas/IfcStructuralElementsDomain/PropertySets/Pset_ConcreteElementGeneral/DocPropertySet.xml

Well, It's solved in the way that these used to be XML strings catted verbatim to the various outputs and now are generated from UML. Whether the lack of a p21 for psets is an issue is another question.