Merging of my Link branch

Here's the place for discussion related to coding in FreeCAD, C++ or Python. Design, interfaces and structures.
Forum rules
Be nice to others! Respect the FreeCAD code of conduct!
Post Reply
User avatar
Kunda1
Veteran
Posts: 13434
Joined: Thu Jan 05, 2017 9:03 pm

Re: Merging of my Link branch

Post by Kunda1 »

It would be important to hear from @wmayer since @yorik already shared some words.
Alone you go faster. Together we go farther
Please mark thread [Solved]
Want to contribute back to FC? Checkout:
'good first issues' | Open TODOs and FIXMEs | How to Help FreeCAD | How to report Bugs
User avatar
looo
Veteran
Posts: 3941
Joined: Mon Nov 11, 2013 5:29 pm

Re: Merging of my Link branch

Post by looo »

Splitting it up to smaller merges would be a terrible waste of time. We can already test, find and solve issues with the complete package of changes and work on it to make it stable enough for all workbenches to merge into master.

No I don't think it's waste of time. If we merge the branch things will break, get worse performance, as already shown... It gets way more difficult to find the root of the problem if all is merged at once. And we are totally dependent on realthunder as he is maybe the only one fully understanding his implementation.
User avatar
fosselius
Posts: 381
Joined: Sat Apr 23, 2016 10:03 am
Contact:

Re: Merging of my Link branch

Post by fosselius »

I was just restating what realthunder said earlier in this thread:
Yes, splitting requires a lot of repeated works, and prone to introducing new bugs. But even if I do this, some feature may still be too big, and there is still no guarantee of more testers. In fact, some new features will be more difficult to test when isolated, unlike now, all the new stuff can be tested in my Assembly3.
I rather see realthunder spending his time on fixing bugs then splitting code. The "merge by small changeset/feature" ship have already sailed more then a year ago...

Time machine time:
Realthunder June 2017
My Link branch has grown way beyond what a few PRs can handle. It needs a lot of tests to make it stable, and I think a working assembly based on link feature is the best way to attract more people to test it. So, here is a pre-announcement. I will start a fork of the assembly2 to take full advantage of the Link feature. Because of the amount of changes expected, I think I'll call it assembly3. Once, I got the basic features working, I'd be very much appreciated for anyone who are interested to contribute as well. I'll start working on this a bit later, though, maybe after a month, since I got caught up with something right now. When I started to work on assembly3, I'll try to keep the branch in sync with upstream master. I hope it will be a memorable milestone of FC when my branch is finally merged, some day!
Then in July 2017
I have divided my branches into three large patch set against the upstream master, and I've just started submitting the first patch set two weeks ago. I'd expect it to take at least a few months to get through, if at all.
Thread:
https://forum.freecadweb.org/viewtopic ... ink+merge

And as we know, the patch sets never got mereged..
realthunder
Veteran
Posts: 2190
Joined: Tue Jan 03, 2017 10:55 am

Re: Merging of my Link branch

Post by realthunder »

looo wrote: Sat Aug 04, 2018 4:30 am
Splitting it up to smaller merges would be a terrible waste of time. We can already test, find and solve issues with the complete package of changes and work on it to make it stable enough for all workbenches to merge into master.

No I don't think it's waste of time. If we merge the branch things will break, get worse performance, as already shown... It gets way more difficult to find the root of the problem if all is merged at once. And we are totally dependent on realthunder as he is maybe the only one fully understanding his implementation.
The problem you and yorik encountered so far are mostly caused by the new topological naming feature. I estimate that feature alone is about 10K lines of changes. This feature is not essential for Link to work, however, because of the existence of Link, this feature becomes even more complex to implement. I actually felt a bit lucky that I am the one who did both, or else if it would be so much more difficult to add the link support on top of an existing topo naming solution.

Anyway, I am currently trace a few problems with your posted wikihouse file. You also mentioned long loading time for a glider model. Is it possible for you to post the file, as well?
Try Assembly3 with my custom build of FreeCAD at here.
And if you'd like to show your support, you can donate through patreon, liberapay, or paypal
User avatar
looo
Veteran
Posts: 3941
Joined: Mon Nov 11, 2013 5:29 pm

Re: Merging of my Link branch

Post by looo »

realthunder wrote:You also mentioned long loading time for a glider model. Is it possible for you to post the file, as well?
This one is solved. I only missed one library...

So also if we go for a big merge, I guess there are some things which can be done in the meantime:
1. Problem with sketcher and topologic-naming that came up here: https://forum.freecadweb.org/viewtopic. ... 0&#p247084
2. Fixing draft. I can confirm problems with this workbench. Somehow it makes a difference if one uses draft after running the tests. The basic stuff works for me without running the tests previously.
3. fixing all the tests (in gui there is one test failing for master and the link-branch?, so actually this should be fixed in master. Which tests are failing for you @yorik?) edit: I think there are some tests passing, but shouldn't regarding the output in the report-view.
4. more testing!!! Maybe we have more big files
triplus
Veteran
Posts: 9471
Joined: Mon Dec 12, 2011 4:45 pm

Re: Merging of my Link branch

Post by triplus »

Links effort got around two FreeCAD development cycles worth of effort invested. Efforts like Assembly 3 were build around it to demonstrate its use. In addition we need Links alike functionality as a part of FreeCAD. Some general and core alike way to reuse (cross document) geometry. The only thing @realthunder didn't tackle directly is currently all external documents need to be opened. For things to work. Therefore this highly likely wont scale in large projects. But it likely will scale in small to mid size projects.

Anyway the plan is to make a release at the end of the year. In my opinion it makes sense to include the Links effort. Not too in-depth review and after merge should happen rather sooner than later. After @realthunder should address reported issues and regressions (FreeCAD 0.17 used as a reference point). Other developers can start exploring Links functionality for Assembly2, Assembly3, A2Plus, BIM, Lattice2 ... purposes.

Therefore what we i guess need now is a PR with core Links effort functionality included. Everything that is strictly needed to make it work. And likely the STEP related work. That makes sense to be included in FreeCAD 0.18. Everything else should be stripped away for now. If that can't be done then there is something wrong here anyway.

P.S. As for the topological naming effort. I don't feel that part is ready and matured enough yet. Therefore i vote for trying again in FreeCAD 0.19 development cycle.
realthunder
Veteran
Posts: 2190
Joined: Tue Jan 03, 2017 10:55 am

Re: Merging of my Link branch

Post by realthunder »

triplus wrote: Sat Aug 04, 2018 12:39 pm @realthunder didn't tackle directly is currently all external documents need to be opened. For things to work. Therefore this highly likely wont scale in large projects. But it likely will scale in small to mid size projects.
Well, while waiting for the response, I've finished implementing the partial loading already. See here. Scroll down to see the screen cast. Once the partial loading is active, the number of files required can be reduced significantly.
Try Assembly3 with my custom build of FreeCAD at here.
And if you'd like to show your support, you can donate through patreon, liberapay, or paypal
User avatar
looo
Veteran
Posts: 3941
Joined: Mon Nov 11, 2013 5:29 pm

Re: Merging of my Link branch

Post by looo »

One Arch test is failing because of:

Code: Select all

Traceback (most recent call last):
  File ".../Mod/Arch/ArchBuildingPart.py", line 373, in updateData
    obj.ViewObject.DiffuseColor = cols
<class 'AttributeError'>: 'Gui.ViewProviderDocumentObject' object has no attribute 'DiffuseColor'
Does the link-branch make any changes to the FeaturePython object?

Also there is this one in the arch tests:

Code: Select all

Arch: error sweeping rebar profile along the base sketch
Traceback (most recent call last):
  File "/home/lo/conda/envs/freecad_assembly/Mod/Arch/ArchBuildingPart.py", line 373, in updateData
    obj.ViewObject.DiffuseColor = cols
<class 'AttributeError'>: 'Gui.ViewProviderDocumentObject' object has no attribute 'DiffuseColor'
Traceback (most recent call last):
  File "/home/lo/conda/envs/freecad_assembly/Mod/Draft/DraftGui.py", line 135, in doTasks
    f(arg)
  File "/home/lo/conda/envs/freecad_assembly/Mod/Arch/ArchWindow.py", line 112, in recolorize
    if obj.ViewObject:
ReferenceError: Cannot access attribute 'ViewObject' of deleted object

During handling of the above exception, another exception occurred:

ReferenceError: Cannot print representation of deleted object

The above exception was the direct cause of the following exception:
And I guess there is a pivy stuff not deleted, which later on results in Draft-tools to be broken.

Although there are these errors reported, arch-tests are passing (green). I guess there is another problem with the reported results. I guess this is not true for master... Maybe also related to py3.
triplus
Veteran
Posts: 9471
Joined: Mon Dec 12, 2011 4:45 pm

Re: Merging of my Link branch

Post by triplus »

realthunder wrote: Sat Aug 04, 2018 1:10 pm
triplus wrote: Sat Aug 04, 2018 12:39 pm @realthunder didn't tackle directly is currently all external documents need to be opened. For things to work. Therefore this highly likely wont scale in large projects. But it likely will scale in small to mid size projects.
Well, while waiting for the response, I've finished implementing the partial loading already. See here. Scroll down to see the screen cast. Once the partial loading is active, the number of files required can be reduced significantly.
I see. Therefore i guess it was worth it. To wait a bit longer and for things to mature further. ;)

Anyway as said in my opinion you should remove the cruft and only make core Links (and STEP) effort related PR. Whatever isn't directy related should be removed. You can always try to upstream that later in small scopes. Without doing that i doubt Werner will do any reviewing. In addition if you will insist to upstream things like Topo Naming effort now. And plethora of other cruft i seen you added to your branch in the past. It could easily happen we will be having this same discussion next August. As i am guessing around one more development cycle of Topo Naming effort is needed to mature things further in that area. When i test Links effort in your branch. In general i get good results. When i test Topo Naming effort in your branch. There is still too little results in my opinion and i rarely see any difference. Compared to stock FreeCAD. That leads me to believe there is still much work to be done in the Topo Naming effort area. And likely like with the Assembly 3 effort you will need to provide more evidence your implementation is sound. That is once some Topo Naming related discussion will emerge on the forum. Your branch will need to provide the answer. Once we get much more of that we can start talking about upstreaming things. But highly likely not before FreeCAD 0.19.

In short last August your Links effort wasn't ready yet to be upstreamed. You did what needed to be done and we can consider doing that now. FreeCAD 0.18 therefore i guess should be Links ready. As for the rest we shouldn't care all that much ATM. Therefore core Links related PR is now needed. Without it it can't get upstreamed. Regardless of all discussions.
User avatar
fosselius
Posts: 381
Joined: Sat Apr 23, 2016 10:03 am
Contact:

Re: Merging of my Link branch

Post by fosselius »

When i test Topo Naming effort in your branch. There is still too little results in my opinion and i rarely see any difference. Compared to stock FreeCAD. That leads me to believe there is still much work to be done in the Topo Naming effort area.
Out of curiosity, how and what did you test?

Your statement is a bit unhelpful, its like "i tested it, it did not work".
A comment like "i tested x,y,z had problem 1 with x, problem 2 with y and problem 3 with z. would be much more helpful.

I do understand that there is much to test and it requires some effort to write down what we are doing and what result we get when we evaluate.
If you are running Linux, Peek can be helpful to record results: https://github.com/phw/peek

here is an example:
i compare fillet handling when it loses its reference between FreeCAD/LinkStage3 left vs FreeCAD/master right (click on the image)
Image
Last edited by fosselius on Sun Aug 05, 2018 2:18 pm, edited 1 time in total.
Post Reply