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

Bever Control Sprint 3.exc #179

Merged
merged 6 commits into from
Mar 4, 2024
Merged

Bever Control Sprint 3.exc #179

merged 6 commits into from
Mar 4, 2024

Conversation

ivaroppen
Copy link
Contributor

@ivaroppen ivaroppen commented Aug 30, 2023

File

A draft for Sprint 3.exc

Covered Concept Templates - Usages Included

The defined consept templates and usages should be included in this draft.

Miscellaneous

@pjanck
Copy link
Member

pjanck commented Sep 16, 2023

@ivaroppen Please sync from main, there is a change necessary to activate the checker.

grafik

If you require assistance, please shoot me an email.

@ivaroppen
Copy link
Contributor Author

ivaroppen commented Sep 16, 2023

Sorry, I need some help with this 2 issues:

Check 'Underground excavation in Sprint 3.exc' with uuid=3e71c130-014e-4a7d-c0de-1fc10ce1deb1
3.e.Exc.1.iv) No spatial containment for underground excavation

The following instances are failing:

#352, with GlobalId '3rVsqBF4j7UvaqqoBZaBIg'

Could you please point me in the direction of what I am doing wrong?

The other one I need help with:

Check 'Element's representation in Sprint 2.3' with uuid=23b70110-014e-4a7d-c0de-1fc10ce1deb1
There were 1 applicable and 0 not applicable IfcUndergroundExcavation(s) in the checked file.

2.3.2.ii) Correct Representation for Element: correct surfaic or volumentric type used. Following may be used: Body tesselation, Body Brep, Body Swept Solid, Surface Model, Voxel, Surface Tesselation

The following instances are failing:

#352, with GlobalId '3rVsqBF4j7UvaqqoBZaBIg'

In sprint 2.3 the allowed geometries does not allow Body Sectioned SolidHorizontal.
However in Sprint3.exc 1.i.a one of the allowed geometries is Body Sectioned SolidHorizontal

The current checker is complaining when I use Body Sectioned SolidHorizontal in Sprint 3.exc (RepresentationType = AdvancedSweptSolid).
The error message indicate it is due to the Sprint 2.3 limitation?

@pjanck
Copy link
Member

pjanck commented Sep 17, 2023

3.e.Exc.1.iv) No spatial containment for underground excavation

#352 is probably contained through IfcRelContainedInSpatialStructure, although this is forbidden. It is sufficient to connect it through IfcRelVoidsElements. I'll double check the checker as well.

The current checker is complaining when I use Body Sectioned SolidHorizontal in Sprint 3.exc (RepresentationType = AdvancedSweptSolid).

Your interpretation is correct. We will update the checker - I'll open an issue for this.

@pjanck
Copy link
Member

pjanck commented Sep 17, 2023

I'll double check the checker as well.

It was the checker as well. I'll update it together with #185 fixes.

@pjanck
Copy link
Member

pjanck commented Oct 13, 2023

It was the checker as well.

I've managed to update the checker - thank you for pointing out the errors in its implementation!

Copy link
Collaborator

@larswik larswik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job with an interesting model! We could merge this with a few comments:

  • IfcTunnelTypicalSection should be contained and not aggregated (maybe fix this?)
  • It seems as if there are two IfcGeoScienceModel (one, PHYSICALDISTRIBUTIONMODEL, aggregating the other, NOTDEFINED) - Intentional?
  • The geometric representation in this case could be made more efficient since all profiles has an equal number of vertices. One IfcSectionedSolidHorizontal with four profiles would do. However, the current representation works. It would be interesting with the widened part having a different number of vertices in the profile. In that case, e.g. an IfcFacetedBrep would be needed in the widening part. It would be interesting though to add guiding curves to the excavation body representation :-)
  • IfcElementAssembly missing PredefinedType=.ARCH.
  • IfcArchElement has PredefinedType USERDEFINED (maybe a software library issue?)
  • IfcArchElement=>Material missing PSet_MaterialConcrete
  • In the IfcBuiltSystem/TUNNEL_SUPPORT, both the IfcElementAssembly and the IfcArchElement (which is decomposing the IfcBuiltsystem) is included
  • The IfcArchElementType and IfcArchElement are not associated. Maybe that's the intention?

@ivaroppen
Copy link
Contributor Author

IfcArchElement has PredefinedType USERDEFINED (maybe a software library issue?)

Yes, I think it’s a library issue.
I created the IfcTunnelTypicalSection in my code.
However, fixing the IfcArchElement require me to do a too deep intervention into the library code....

The geometric representation in this case could be made more efficient <

Agree, however, I did not fix it this time.

The IfcArchElementType and IfcArchElement are not associated. Maybe that's the intention?

Sorry, did not fix in this time.

The remaining points in the review should be fixed in this version…

@SergejMuhic
Copy link
Collaborator

IfcArchElement has PredefinedType USERDEFINED (maybe a software library issue?)

Yes, I think it’s a library issue. I created the IfcTunnelTypicalSection in my code. However, fixing the IfcArchElement require me to do a too deep intervention into the library code....

The workaround is quite nice though. I can offer a re-serialization with IFC Reactor to make it better but it would modify your header FILE_NAME content to a different processor. Let me know whether you would like me to do that.

It is turning out to be quite the "beacon file".

Predefined type for IfcArchElement
@ivaroppen
Copy link
Contributor Author

ivaroppen commented Nov 26, 2023

Please see #167 for an explanation on the guidelines.
As agreed, the guidelines are not lines in 3D space:
They describe how a single parameter varies along the peg number axis (offset y, offset z and radius)...
The example is a real T9,5 to T12,5 transition, ie. it is not fake data....

@larswik
Copy link
Collaborator

larswik commented Jan 30, 2024

@SergejMuhic : I think that the guided curve implementation needs to be discussed/resolved. The way I understood it, we discussed having one element (e.g. IfcUndergroundExcavation) having two geometric representations (e.g. IfcSectionedSolidHorizontal + IfcGeometricCurveSet for the guide curves). In the provided dataset, the sectioned solid belongs to the excavation elemennt and the curve set to a separate spatial zone. Furthermore, i wonder what the below means practically in the IFC file with regards to e.g. contexts:
"As agreed, the guidelines are not lines in 3D space:
They describe how a single parameter varies along the peg number axis (offset y, offset z and radius)..."

Also, I find the documentation for IfcPolynomialCurve sparse, making it unclear how to use the coefficients.

@SergejMuhic
Copy link
Collaborator

We should create a usage for it. We have the resolution documented in issues bSI-InfraRoom/IFC-Specification#118 and bSI-InfraRoom/IFC-Specification#648.

I have created an issue to resolve the applicable entities:
bSI-InfraRoom/IFC-Specification#730

@pjanck pjanck merged commit c30932e into bSI-InfraRoom:main Mar 4, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants