freecad.gears icon indicating copy to clipboard operation
freecad.gears copied to clipboard

Internal Involute Gear - Helical Mode errors

Open scottmudge opened this issue 2 years ago • 16 comments

When I apply a beta angle to the new internal involute gear, the following errors show up in the report panel:

09:09:57  <ComplexGeoData> features.py(1573)|ComplexGeoData.cpp(1622): unresolved duplicate element mapping 'Edge1;SHL Edge501/Edge297
09:09:57  <ComplexGeoData> features.py(1573)|ComplexGeoData.cpp(1622): unresolved duplicate element mapping 'Edge2;SHL Edge503/Edge299
09:09:57  <ComplexGeoData> features.py(1573)|ComplexGeoData.cpp(1622): unresolved duplicate element mapping 'Edge3;SHL Edge504/Edge300
09:09:57  <ComplexGeoData> features.py(1573)|ComplexGeoData.cpp(1622): unresolved duplicate element mapping 'Edge4;SHL Edge505/Edge304
09:09:57  <ComplexGeoData> features.py(1573)|ComplexGeoData.cpp(1622): unresolved duplicate element mapping 'Face1;SHL Face168/Face100
09:09:57  <ComplexGeoData> features.py(1573)|ComplexGeoData.cpp(1622): unresolved duplicate element mapping 'Vertex1;SHL Vertex337/Vertex199
09:09:57  <ComplexGeoData> features.py(1573)|ComplexGeoData.cpp(1622): unresolved duplicate element mapping 'Vertex2;SHL Vertex338/Vertex200
09:09:57  <ComplexGeoData> features.py(1573)|ComplexGeoData.cpp(1622): unresolved duplicate element mapping 'Vertex3;SHL Vertex3/Vertex203
09:09:57  <ComplexGeoData> features.py(1573)|ComplexGeoData.cpp(1622): unresolved duplicate element mapping 'Vertex4;SHL Vertex4/Vertex204
09:09:57  <ComplexGeoData> features.py(1573)|ComplexGeoData.cpp(1622): unresolved duplicate element mapping 'Edge1;SHL Edge504/Edge297
09:09:57  <ComplexGeoData> features.py(1573)|ComplexGeoData.cpp(1622): unresolved duplicate element mapping 'Edge2;SHL Edge506/Edge299
09:09:57  <ComplexGeoData> features.py(1573)|ComplexGeoData.cpp(1622): unresolved duplicate element mapping 'Edge3;SHL Edge4/Edge300
09:09:57  <ComplexGeoData> features.py(1573)|ComplexGeoData.cpp(1622): unresolved duplicate element mapping 'Edge4;SHL Edge507/Edge304
09:09:57  <ComplexGeoData> features.py(1573)|ComplexGeoData.cpp(1622): unresolved duplicate element mapping 'Face1;SHL Face169/Face100
09:09:57  <ComplexGeoData> features.py(1573)|ComplexGeoData.cpp(1622): unresolved duplicate element mapping 'Vertex1;SHL Vertex339/Vertex199
09:09:57  <ComplexGeoData> features.py(1573)|ComplexGeoData.cpp(1622): unresolved duplicate element mapping 'Vertex1;SHL Vertex340/Vertex199
09:09:57  <ComplexGeoData> features.py(1573)|ComplexGeoData.cpp(1622): unresolved duplicate element mapping 'Vertex2;SHL Vertex341/Vertex200
09:09:57  <ComplexGeoData> features.py(1573)|ComplexGeoData.cpp(1622): unresolved duplicate element mapping 'Edge1;SHL Edge508/Edge297
09:09:57  <ComplexGeoData> features.py(1573)|ComplexGeoData.cpp(1622): unresolved duplicate element mapping 'Edge1;SHL Edge509/Edge297
09:09:57  <ComplexGeoData> features.py(1573)|ComplexGeoData.cpp(1622): unresolved duplicate element mapping 'Face1;SHL Face171/Face100

I am using the following parameters:

image

The gear looks fine, but it fills up the report. There are dozens of the same entries every time the object is recomputed.

scottmudge avatar Dec 01 '21 14:12 scottmudge

can you post your freecad-version. I cannot reproduce with:

OS: Ubuntu 18.04.6 LTS (ubuntu:GNOME/ubuntu)
Word size of FreeCAD: 64-bit
Version: 0.20.26498 (Git)
Build type: Release
Branch: (HEAD detached at 1895593)
Hash: 18955931c7f0926a4cd9d2719be5a433b49ae56e
Python version: 3.9.7
Qt version: 5.12.9
Coin version: 4.0.0
OCC version: 7.5.3
Locale: German/Germany (de_DE)

looooo avatar Dec 01 '21 14:12 looooo

OS: Windows 10 Version 2009 Word size of FreeCAD: 64-bit Version: 0.20.26244 +4375 (Git) Build type: Release Branch: LinkDaily Hash: 4515938ac0b697bc41f8a5dbd3ad91df92eb3f8d Python version: 3.8.6+ Qt version: 5.15.2 Coin version: 4.0.1 OCC version: 7.5.0 Locale: English/United States (en_US)

Using Realthunder's branch. I compiled from the LinkDaily tip yesterday, but it might happen on his current 10.15 release as well, I'm not sure.

scottmudge avatar Dec 01 '21 14:12 scottmudge

Let me check out an older node on his fork, just to rule out any recent changes have caused issues.

scottmudge avatar Dec 01 '21 14:12 scottmudge

Okay so I can confirm it occurs on pretty much any recent release on Realthunder's fork:

Example - https://github.com/realthunder/FreeCAD_assembly3/releases

I am assuming a lot of this fork is slated to be merged into 0.20 before the final release, so the issue may eventually be migrated into the official FreeCAD fork.

@realthunder Is this an issue with this particular module or is there some other incompatibility?

scottmudge avatar Dec 01 '21 14:12 scottmudge

More updates:

  1. It happens only for gears with > 24 teeth (i.e., 25 or more teeth).
  2. It happens only for non-zero beta value.
  3. Other parameters appear not to affect it.

scottmudge avatar Dec 01 '21 16:12 scottmudge

Okay, one more update.

So the error being thrown in ComplexGeoData::setElementName() looks like it's being triggered by an arbitrarily set max iteration threshold:

image

I think I know what is happening. Because this is only triggered at 25 teeth, each tooth is (presumably) creating 4 elements. 4 * 25 = 100, leading to the threshold being hit.

@realthunder -- Why is this threshold set to 100? For performance reasons? Is there any reason not to remove it?

scottmudge avatar Dec 01 '21 16:12 scottmudge

A while back it looks like it was set to a while(1) loop, so it looks like this threshold was put in place as sort of a sanity check to prevent the software from freezing.

Unfortunately it looks like a different break condition might need to be used.

scottmudge avatar Dec 01 '21 16:12 scottmudge

Increasing if (++i == 100) to if (++i == 4096) fixed the issue, but I'm not sure if it is the ideal solution or not.

I stress-tested it by creating a 1000-tooth internal involute gear. Increasing the break threshold to 4096 prevented all of the errors, but I feel like it also took longer to complete generation (~45 seconds), while leaving the threshold at 100 completed around 25-30 seconds.

scottmudge avatar Dec 01 '21 17:12 scottmudge

Added this issue to FreeCAD project, as it will likely need some fixes there as well.

But perhaps there is a way to refactor helicalextrusion() to avoid having to use Part.makeShell()?

I also noticed that when using Face binders with gears, I can predictably bind the top and bottom faces of gears when they are NOT helical gears, even when the number of teeth are changed. However, when the gear is made helical, the face binder breaks, as the upper and lower faces change.

To correct it, the face binder has to be manually re-set to the correct upper and lower faces. And if the teeth count changes while it is still helical, the face binder will have to be re-set every single time. But again, this is only when it is helical. When beta is set to 0, the upper and lower faces are always the same, and the face binder works as expected.

scottmudge avatar Dec 01 '21 20:12 scottmudge

This 'non fatal' error is due to failure in auto resolving topo name duplicates. The threshold is added because It is potentially expensive to auto resolve as each geometry element produced by each operation needs to have a name whenever possible. The intention is to remind the programmer to insert op code when making shapes to help disambiguation. The log message shows the error is triggered in features.py:1573, but I can't seem to find any code relevant there. Which branch are you using?

realthunder avatar Dec 02 '21 01:12 realthunder

Line 1573 in features.py for me was:

    shell = Part.makeShell(shell_faces)

in def helicalextrusion(face, height, angle, double_helix=False). makeShell() presumably produces a call stack leading to ComplexGeoData::setElementName().

I am using an older node (ef73b6604083237fb815841d67c02643dbede8d2) from the develop branch, but the issue persists on the current tip of develop as well.

What is the effect of the error, even if it is non-fatal? I am using the affected face produced by the gear as a parametric input downstream, so I need the integrity of the element relationships to be in-tact.

If it's simply element naming (with no effect on integrity of the part's relationship), then that's fine. But perhaps it should be switched to warning level?

In this case, it will be triggered any time an internal gear is produced with 25 or more teeth. Which is incredibly common for internal gears.

Would a higher threshold be more appropriate? Or is makeShell() being inappropriately used?

scottmudge avatar Dec 02 '21 01:12 scottmudge

I also noticed that when using Face binders with gears, I can predictably bind the top and bottom faces of gears when they are NOT helical gears, even when the number of teeth are changed. However, when the gear is made helical, the face binder breaks, as the upper and lower faces change.

To correct it, the face binder has to be manually re-set to the correct upper and lower faces. And if the teeth count changes while it is still helical, the face binder will have to be re-set every single time. But again, this is only when it is helical. When beta is set to 0, the upper and lower faces are always the same, and the face binder works as expected.

Just a suggestion for you to work around the issue with the face builder: Depending on what you need the face binder for, it my be an option to just use ordinary circles that reference (via expressions) the inner/outer diameter of the gear object, offset by the gear's height.

jbaehr avatar Dec 02 '21 21:12 jbaehr

I do use that approach for aligning objects, but I'm actually creating a lofted transition between two gears of different teeth count, so using face binders (or referenced faces) is necessary to get the surface profiles. For now I'm just using non-helical gears, but should I want to use helical gears in the future, it's looking like I'll have to get rid of the lofted transition, and instead just use a regular planar divider (or no divider).

scottmudge avatar Dec 02 '21 21:12 scottmudge

I just fixed an obscure bug in sub shape topo naming, which may be the root cause of the duplicated element error message here. You may want to pull my branch and try again.

realthunder avatar Dec 14 '21 06:12 realthunder

Yep, this is fixed now, with leaving if (++i==100) in place. Thanks!

scottmudge avatar Dec 14 '21 08:12 scottmudge

I'm sorry, I had a "brain fart". When I rebuilt and tested with your latest commits @realthunder, I went back to double check, and unfortunately I did not actually revert my change to if (++i == 2048), so the reason I did not see the problem was because of this higher threshold.

However, the issue persists still:

many more
...
15:45:30  <ComplexGeoData> features.py(1653)|ComplexGeoData.cpp(1624): unresolved duplicate element mapping 'Vertex1;SHL Vertex385/Vertex199
15:45:30  <ComplexGeoData> features.py(1653)|ComplexGeoData.cpp(1624): unresolved duplicate element mapping 'Vertex2;SHL Vertex386/Vertex200
15:45:30  <ComplexGeoData> features.py(1653)|ComplexGeoData.cpp(1624): unresolved duplicate element mapping 'Vertex3;SHL Vertex3/Vertex203
15:45:30  <ComplexGeoData> features.py(1653)|ComplexGeoData.cpp(1624): unresolved duplicate element mapping 'Vertex4;SHL Vertex4/Vertex204
...
many more

scottmudge avatar Dec 14 '21 20:12 scottmudge