Ifcopenshell: Layerset folding broken with duplex in v0.6

[otsoa]

The breaking commit seems to be: 75b24c2352 Put maximum length on layerset wall end point intersection
Removing the first condition d < total_thickness * 3 in following seems to fix it for this model:

if (d < total_thickness * 3 && d < mind) {
    GeomAdaptor_Curve GAC(other_axis_curve);
    GeomAdaptor_Surface GAS(kt->first);

    Extrema_ExtCS x(GAC, GAS, getValue(GV_PRECISION), getValue(GV_PRECISION));

    if (x.IsParallel()) {
        body_surface = kt->first;
        layer_body_intersection = kt->second;
        mind = d;
    }
}

[Francois-Thierry]

Hi otsoa,
this issue was fixed on September 20th 2020 (see https://github.com/IfcOpenShell/IfcOpenShell/issues/996).
Maybe consider upgrading IfcOpenShell ?
Regards
François

[otsoa]

I just tested with latest v0.6.0 (e62b7e94a4 2021-03-08 16:42) and the issue seems to be there. Maybe it's regression?

[Francois-Thierry]

Yes you are right, it is a regression.
Here is the commit that broke it : https://github.com/IfcOpenShell/IfcOpenShell/commit/acb518613bb93b1370941c0fdaf7796d11eec2a5

[otsoa]

fold_layers is pretty complex function. I wonder if it was possible to write some unit tests for it with f.e. GoogleTest framework. Probably they would save time in the end. I might take a look into that if I have time. @aothms How do you see it? Worth to try?
Actually property based testing with something like https://github.com/emil-e/rapidcheck might be a good fit. There are probably some geometric properties or symmetries that must hold for all input and output.

[aothms]

@otsoa definitely! It's schema dependent code so maybe that's a
complicating factor. But I'm definitely in favor of this. Same for the
parsing btw. If you integrate gtest in cmake I'll see if I can contribute
some tests as well.

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

Op vr 12 mrt. 2021 08:43 schreef otsoa @.*>:

fold_layers is pretty complex function. I wonder if was possible to write
some unit tests for it with f.e. GoogleTest framework. Probably they would
save time in the end. I might take a look into that if I have time.
@aothms https://github.com/aothms How do you see it? Worth to try?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/IfcOpenShell/IfcOpenShell/issues/1368#issuecomment-797301376,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAILWV2UWP54PXBMV32T73TTDHA2VANCNFSM4Y3MZG4Q
.

[aothms]

Thanks for tracking this down @otsoa and @Francois-Thierry It was indeed introduced at https://github.com/IfcOpenShell/IfcOpenShell/commit/acb518613bb93b1370941c0fdaf7796d11eec2a5 which was a fix for #1118 instead of looking for parallel walls the fix should have been to see if the length of the wall can actually accommodate the folds.

[Francois-Thierry]

Hi,
I can confirm that the regression is fixed as of 624e296.
Thanks for the quick resolve !
Kind Regards
François

[otsoa]

Great job!