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

Internal Involute Gear - Helical Mode errors #90

Open
scottmudge opened this issue Dec 1, 2021 · 16 comments
Open

Internal Involute Gear - Helical Mode errors #90

scottmudge opened this issue Dec 1, 2021 · 16 comments

Comments

@scottmudge
Copy link
Contributor

scottmudge commented Dec 1, 2021

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.

@looooo
Copy link
Owner

looooo commented Dec 1, 2021

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)

@scottmudge
Copy link
Contributor Author

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
Copy link
Contributor Author

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

@scottmudge
Copy link
Contributor Author

scottmudge commented Dec 1, 2021

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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.

@realthunder
Copy link
Contributor

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?

@scottmudge
Copy link
Contributor Author

scottmudge commented Dec 2, 2021

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 (ef73b66) 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?

@jbaehr
Copy link
Contributor

jbaehr commented Dec 2, 2021

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.

@scottmudge
Copy link
Contributor Author

scottmudge commented Dec 2, 2021

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).

realthunder added a commit to realthunder/FreeCAD that referenced this issue Dec 14, 2021
@realthunder
Copy link
Contributor

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.

@scottmudge
Copy link
Contributor Author

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

@scottmudge
Copy link
Contributor Author

scottmudge commented Dec 14, 2021

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 scottmudge reopened this Dec 14, 2021
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

4 participants