Merging of my Link branch
Forum rules
Be nice to others! Respect the FreeCAD code of conduct!
Be nice to others! Respect the FreeCAD code of conduct!
Re: Merging of my Link branch
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
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
Re: Merging of my Link branch
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.
Re: Merging of my Link branch
I was just restating what realthunder said earlier in this thread:
Time machine time:
Realthunder June 2017
https://forum.freecadweb.org/viewtopic ... ink+merge
And as we know, the patch sets never got mereged..
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...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.
Time machine time:
Realthunder June 2017
Then in July 2017My 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!
Thread: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.
https://forum.freecadweb.org/viewtopic ... ink+merge
And as we know, the patch sets never got mereged..
-
- Veteran
- Posts: 2190
- Joined: Tue Jan 03, 2017 10:55 am
Re: Merging of my Link branch
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.looo wrote: ↑Sat Aug 04, 2018 4:30 amSplitting 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.
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?
Re: Merging of my Link branch
This one is solved. I only missed one library...realthunder wrote:You also mentioned long loading time for a glider model. Is it possible for you to post the file, as well?
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
Re: Merging of my Link branch
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.
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.
-
- Veteran
- Posts: 2190
- Joined: Tue Jan 03, 2017 10:55 am
Re: Merging of my Link branch
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.
Re: Merging of my Link branch
One Arch test is failing because of:
Does the link-branch make any changes to the FeaturePython object?
Also there is this one in the arch tests:
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.
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'
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:
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.
Re: Merging of my Link branch
I see. Therefore i guess it was worth it. To wait a bit longer and for things to mature further.realthunder wrote: ↑Sat Aug 04, 2018 1:10 pmWell, 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.
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.
Re: Merging of my Link branch
Out of curiosity, how and what did you test?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.
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)
Last edited by fosselius on Sun Aug 05, 2018 2:18 pm, edited 1 time in total.