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

IFC4X4 issues #807

Open
iegorychev opened this issue May 27, 2024 · 1 comment
Open

IFC4X4 issues #807

iegorychev opened this issue May 27, 2024 · 1 comment

Comments

@iegorychev
Copy link
Collaborator

Hello, found following:

  1. Double semicolon appears in current IFC4X4 schema sometimes in such places:
    Values : ARRAY [1:SELF\IfcVoxelData.GridSize] OF IfcInteger := IfcListToExpandedArray(ValueData,1,SELF\IfcVoxelData.GridSize,SELF\IfcProduct.Representation.Representations[1].Items[1]\IfcVoxelGrid.Voxels);;
    This looks incorrect from the point of view of grammar.

  2. Voxels : ARRAY [1:NumberOfVoxelsXNumberOfVoxelsYNumberOfVoxelsZ] OF IfcBoolean;
    Again, NumberOfVoxelsX (or NumberOfVoxelsY and NumberOfVoxelsZ) are unset, so ARRAY can not be instantiated as upper bound is indeterminate.
    Looks like LIST or combination of LIST+ARRAY.

  3. CorrectGridSize : SIZEOF(SELF\Voxels) = NumberOfVoxelsX * NVL(NumberOfVoxelsY, NumberOfVoxelsX) * NVL(NumberOfVoxelsZ, NumberOfVoxelsX);

SIZEOF(SELF\Voxels) - doesn't look as qualified_attribute (SELF\entity_ref.attribute_ref), seems should be just SIZEOF(Voxels) as this isn't redeclared (as in other places within schemas).

@SergejMuhic
Copy link
Collaborator

To 1, that semicolon slipped through because I tested the expression only and IfcDoc when generating the schema automatically ads it. Fixes here.
To 2, this is allowed according to grammar. Checked and validated with an author of EXPRESS:
image
there will be an update to this part though, if you mean the optional version of "NumberOfVoxelsY and NumberOfVoxelsZ". NumberOfVoxelsX is set since it is not optional. Could go either way here, calling NVL or making them mandatory. I lean towards the latter i. e. making them mandatory.
To 3, Indeed, have we not had this issue already? Apparently with the merges it got reverted.

Thanks. Will correct and commit. But please wait for the merges as two PRs are still open. After I merge, I will also run the intepreter.

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

No branches or pull requests

2 participants