Solver/CGS architecture changes: Request for comment

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!
abdullah
Veteran
Posts: 4935
Joined: Sun May 04, 2014 3:16 pm
Contact:

Re: Solver/CGS architecture changes: Request for comment

Post by abdullah »

DeepSOIC wrote:
abdullah wrote:Have you taken a look at DeepSOICs proposal? Does it collide with your code?
I'm sure it does. I have no problem redoing my changes to the new architecture, given some time to get into it. My changes are not that big, I even think that they are too small to be called "architecture changes". The only question is - when we get to that point - will I have time or even desire to do so?... I can't be sure.

EDIT: now that I've bought a new huge touchscreen monitor for my development PC :mrgreen: , I'll probably shift interest into bringing touchscreen support into FreeCAD.
Small collision are not a problem. Concept collision are.

You propose some architectural changes. Johan proposes others. If we do not talk, we risk major incompatibilities and headless direction.

This does not and should not be a problem that takes ages to be decided upon. If not the project itself will stall because of loss of motivation/disappointment of developers. Regaining loss motivation is very difficult. I am only trying to bring on the same table the key players (to my knowledge), so that we can discuss and decide (well Werner will decide).

We can not continue using the name freegcs, this has an easy solution, change it. Ellipse suppport is also at stake because logari does not want internal alignment in it. So if we want to push ellipse support in, we have to change the name. Johan has modifications on logari's code. I want to facilitate the transition for everyone. Most probably Johan's code would nicely rebase on a master including ellipse, because I kept the architecture, but ellipse can not keep the library name. So here the decision is afaiu:

master - change of name - ellipse - johan -DeepSoIC or
master - johan - change of name - ellipse - DeepSOIC

That is what I am asking Johan to evaluate.

Additionally, I would not like to keep two cgs simultaneously in FC. We should develop one as a community.
User avatar
DeepSOIC
Veteran
Posts: 7896
Joined: Fri Aug 29, 2014 12:45 am
Location: used to be Saint-Petersburg, Russia

Re: Solver/CGS architecture changes: Request for comment

Post by DeepSOIC »

Johan! Would you be so kind to explain your changes?
In particular:
* looks like you are bringing user-expression driven constraints (AWESOME ;) ). But how is it organized from user's point of view?
* what is the state of your code? Is it working? Is it ready for merging? Can I compile and see it in action?
* I've seen the keyword "reentrant", which makes me think you are planning multithreading. But I haven't spotted the relevant changes to the solver. What about that? If you've already done the multithreading, I'm very curious to know how it was paralleled. Or if not - how was it intended to be (e.g. different solvers running in different threads).
User avatar
DevJohan
Posts: 41
Joined: Sun Jul 13, 2014 2:36 pm
Location: Stockholm, Sweden

Re: Solver/CGS architecture changes: Request for comment

Post by DevJohan »

Hi everyone.

To start out with I've been kind of busy with work and other stuff these last couple of months so I have not done any coding and I'm not up to speed on the latest developments in the sketcher with ellipses, parabola and hyperbola. I also don't remember my every every design choice I made back in aug-sep so I have to freshen up before I can answer all of your questions.

I also want to add a caveat as my experience coding in teams are quite limited. My main coding experience is in writing fairly optimized code aimed at for example image processing in C++. There the architecture is fairly simple but the code should compile to high performance machine-code. In my experience templates can be a great tool to let the compiler know a lot of information about the actual workings of the code without obscuring the code from my perspective. I have my favorite way of writing code, as I think every programmer has, and mine involves using a lot of variadic templates 8-). To see why this is the case check out http://channel9.msdn.com/Events/GoingNa ... re-Funadic :lol:. Ok, jokes aside, I'm not only here to code and use FreeCAD but also to learn. I you feel I've made some strange decisions please let me know so that I can improve my coding or explain why I prefer the way I wrote the code.

I think some clarifications about my sketchdevel branch is in place.

Currently, there are two solvers working in parallel, a (fairly?) unmodified freegcs and my re-factored variant freegcs_exp ( logari81, what does gcs stand for btw? ). Both provide their own solutions which are drawn while modifying the sketch through dragging around the elements but once you finalize your editing only the result from the main solver is kept. This means you can evaluate the difference of the two solvers both runtime wise but also how they deal with those extra amounts of freedom that allows the solution to vary. To make this possible I added an interface to the solver called SketchSolver which is common to both freegcs and freegcs_exp.

To visualize and keep track of the time taken to solve the sketch I added a widget currently titled "Solver status history" which displays the time taken in a quasi-logarithmic fashion. This widget is a total hack but serves an important function while developing the sketcher. The reported time in "Solver messages" I found to be pretty useless since the solver is applied a couple of times for each change to the sketch and only the last solve time is ever visible. This tends to report a much lower time than is actually required to solve the sketch.

When it comes to multi-threading I've not done anything solid yet. My main ambition in this field is to run the solver in a thread separate from the event thread as it is done today. I don't think it's a good idea for the GUI to become unresponsive while solving the sketch and keeping the solver in a separate thread would not only keep responsiveness but also allow you to abort trying to solve the sketch if you realize you did something stupid. As things are today, if I add a conflicting constraint it is often quicker to kill FreeCAD and load my model again than to wait for the solver to go through the maximum number of iterations :?.

Right now the expressions are really in an embryonic state and is never used inside the other code. It just sits there as an appendix waiting to be developed and incorporated into the other parts of the code. At first I guess it will act as a wrapping of Quantity to allow use of unspecified quantities. I think when updating the architecture of the solver I believe It is important to consider how to do integrations of expressions as these allow for some really powerful additions to the sketcher but requires some special care since they introduce separate unknowns into the constraints. The biggest problem here is to allow expressions without totally destroying the performance of the solver.

DeepSOIC, my current idea of how the expressions are supposed to work from a user standpoint is when you enter a datum for some constraint you can use not only quantity values but also variable names. Named variables are automatically created once referred and if you enter the same variable name in multiple constraints those data are implicitly related. In the task section named variables should be displayed along with their definition (if given) and the value they currently evaluate to. I guess along with the constraints and element widgets we would have an expressions widget which organizes the extra expressions of the sketch.

Unfortunately I have yet to go through the changes made by DeepSOIC in his branch. Which leaves me with a question which branch I sholud rebase to if I were to do that?

This post is long enough as it is (in real abdullah style almost ;)) so I will leave the stage open.

Cheers,
Johan
User avatar
DeepSOIC
Veteran
Posts: 7896
Joined: Fri Aug 29, 2014 12:45 am
Location: used to be Saint-Petersburg, Russia

Re: Solver/CGS architecture changes: Request for comment

Post by DeepSOIC »

Nice to hear all that stuff! My comments.
DevJohan wrote:( logari81, what does gcs stand for btw? )
My guess would be "Geometric Constraint Solver".
DevJohan wrote: Both .. solutions .. are drawn while .. dragging .... . To make this possible I added an interface to the solver called SketchSolver which is common to both freegcs and freegcs_exp.
Looks like it is for experimentation purposes, and there's no point integration that stuff into master. Right?
DevJohan wrote:To visualize and keep track of the time taken to solve the sketch I added a widget currently titled "Solver status history"
I did similar thing, but it was simply logging iteration counts to the console. And I found something very interesting: the number of iterations typically is around 8-15, jumping up to 30-50 when the convergence is to a double root. It almost never exceeded 100 (except when it hangs of course), so I've set the fixed iteration count limit (=100) and got rid of those hangs at practically no cost. You may note that I have put that in as the very first commit, because the improvement is so substantial I can't live without it anymore :mrgreen: .
DevJohan wrote:When it comes to multi-threading I've not done anything solid yet. My main ambition .. is to run the solver in a thread separate from the event thread.... ... allow you to abort trying to solve the sketch .
I've done such an interruption in my previous windows-only programming by simply checking GetKeyState(Esc). I don't know if there's such a function in Qt, but it is the simplest IMO. + see previous comment. Threading looked very promising at first (to me), but now I feel that it will take massive amount of work with not-so-great result.
DevJohan wrote:DeepSOIC, my current idea of how the expressions are supposed to work from a user standpoint is when you enter a datum for some constraint you can use not only quantity values but also variable names. Named variables are automatically created once referred and if you enter the same variable name in multiple constraints those data are implicitly related. In the task section named variables should be displayed along with their definition (if given) and the value they currently evaluate to. I guess along with the constraints and element widgets we would have an expressions widget which organizes the extra expressions of the sketch.
I also thought on the problem. I thought to make a special "Constraint on constraints", where one would enter an expression defining a relationship between datum values of named constraints. It looks like being much easier to implement at least, but it may not be as powerful. I also thought it may be a good idea to make a datum display constraint, which would not constrain anything, but simply display a corresponding value instead (that is required to implement the "constraint on constraints" idea.
DevJohan wrote:which branch I sholud rebase to if I were to do that?
I'm afraid I can't answer the question, since looks like I'll have to change everything because of the renaming of the solver. But now my latest branch is EllipseAngleViaPoint, based on a slightly outdated state of Abdullah's ellipse branch. I'm waiting for the ellipse to be merged into master.
DevJohan wrote:This post is long enough as it is (in real abdullah style almost )
:lol: :lol: :lol:
abdullah
Veteran
Posts: 4935
Joined: Sun May 04, 2014 3:16 pm
Contact:

Re: Solver/CGS architecture changes: Request for comment

Post by abdullah »

I am honoured that you have given my name to a posting style :lol:
DevJohan wrote: I have my favorite way of writing code, as I think every programmer has, and mine involves using a lot of variadic templates
Cool!! I will order a brain upgrade to understand that code!! :-)
DevJohan wrote:To make this possible I added an interface to the solver called SketchSolver which is common to both freegcs and freegcs_exp.
Then, regarding FreeCADCGS (we could also call it CCGS (i.e. Community CGS)), I guess, it is more of a matter of making any change compatible with that interface.

A separate issue is then with respect to what you compare your improvements. I would preliminarily suggest that you compare your exp performance against the new FreeCADCGS. DeepSOIC's modifications, deal mainly, afaiu, with simplifying the calling of the constraints by using polymorphism, and a new constraint. From my standpoint, it seems to me that this concrete aspect should not collide much as you are centered on the solver and he is centered in the constraints, where one constraint more is added. It might require that you update your "exp" version to account for that extra constraint.

AFAIU, this "two solver" thing is a development step more than a definitive one. You are trying to measure the improvement of your implementation with respect to that one existing. So at the end, when your changes to master are ready, you will add all your improvements to the "FreeCADCGS" (or CCGS or whatever), and in master only one solver/CGS architecture will be present.
DevJohan wrote:DeepSOIC, my current idea of how the expressions are supposed to work from a user standpoint is when you enter a datum for some constraint you can use not only quantity values but also variable names. Named variables are automatically created once referred and if you enter the same variable name in multiple constraints those data are implicitly related. In the task section named variables should be displayed along with their definition (if given) and the value they currently evaluate to. I guess along with the constraints and element widgets we would have an expressions widget which organizes the extra expressions of the sketch.
I quite agree on the approach. It goes in the same direction I had in mind. A wrapper of Quantity+A manager that apart from organization also arbitrates/decides on the order on how to apply changes (solving order). Mine is just an idea in my head without a well-defined form yet.

DeepSOIC, at the end that is a "Constraint on constraints", but trying to keep it flexible and not overload the UI with specific operations. Here I am more for an approach of "string parsing" than a defined number of UI operations. When you go to this level, I think you generally would like to rely on mathematics more than in "mechanical" approaches (though this can be abstracted in some UI implementation if necessary). Just an opinion.
DevJohan wrote:Unfortunately I have yet to go through the changes made by DeepSOIC in his branch. Which leaves me with a question which branch I sholud rebase to if I were to do that?
So, all in all:
A. DevJohan does not have code to contribute at this time (unless I missed something). So the objective of all the modifications we do from his point of view would be to simplify his rebasing as much as possible.

B. Ellipse is in pull request state, but it needs the change in naming to comply with the naming requirements.

C. DeepSOIC's work needs the Ellipse implementation merged.

The changes to be made (renaming) affect to Ellipse and DeepSOIC's work. Given that A is not ready, and B and C are ready and share a common starting point, my proposal would be to:

PROPOSAL
========
1. Integrate DeepSOIC modifications into Ellipse, and then change the name to the library to the new name. Then DevJohan would need to rebase on master after that is pushed.

If someone has a better approach please tell us.

2. Change the name: I propose CGCS for shortness (Community Geometric Constraint Solver).

If you vote positively to this proposal (please vote either negatively or positively and propose alternatives where it applies), I will dedicate all my free time next week to do the integration (with your help DeepSOIC ;) ).

Does this sound reasonable? If it is not, please say it now before I invest more time in it.
jmaustpc
Veteran
Posts: 11207
Joined: Tue Jul 26, 2011 6:28 am
Location: Australia

Re: Solver/CGS architecture changes: Request for comment

Post by jmaustpc »

I don't want to see one developer slowed by other work/developers where at all possible not to. I think that is highly de-motivating....and I suppose it can also be a little frustrating for users...

Abullah's work is already at pull request state.
I have no idea if Werner will accept it as is or with little modifications or not. If he will accept it with or without minor mods...then we should let that go into master, then these other related pieces of work should be discussed, asset developed to pull request state.

Regarding the FreeCGS name, if it is developed for specifically FreeCAD, which it is as far as I know at this time, then I would suggest it be named FreeCADCGS. Others can rename it if they wish to fork it for some other project. But I don't have a strong opinion on the matter.

If the name is to be changed then I would have thought it would be most appropriate to make a pull request that only changes the name. Thus making life easier for the integrator. If that is to be done then can't it be done after Ellipse pull request has been dealt with?
User avatar
DeepSOIC
Veteran
Posts: 7896
Joined: Fri Aug 29, 2014 12:45 am
Location: used to be Saint-Petersburg, Russia

Re: Solver/CGS architecture changes: Request for comment

Post by DeepSOIC »

abdullah wrote:PROPOSAL
========
Integrate DeepSOIC modifications into Ellipse, and then change the name to the library to the new name.
Sounds good to me. I wonder what Werner thinks.
You didn't mention pull requests here. Did you mean: abort pending ellipse pull request -> join my stuff -> rename library -> new pull request?
User avatar
DeepSOIC
Veteran
Posts: 7896
Joined: Fri Aug 29, 2014 12:45 am
Location: used to be Saint-Petersburg, Russia

Re: Solver/CGS architecture changes: Request for comment

Post by DeepSOIC »

As for the naming, my own preference is flatGCS, because there will be other instances of GCS in FreeCAD, one in assembly wb (EDIT: no, it's a new solver) and (possibly, in the future) one in 3d sketcher.
EDIT: it looks like for assembly wb, a completely new solver is being made.
abdullah
Veteran
Posts: 4935
Joined: Sun May 04, 2014 3:16 pm
Contact:

Re: Solver/CGS architecture changes: Request for comment

Post by abdullah »

What I meant was exactly that. It depends how we do it.

1. We rename before ellipse is merged => I have to rebase to that and you too
2. We rename inbetween => You have to rebase it
3. We rename just after both codes are merged => None of us has to rebase it

It does not necessarily mean that we cancel the pull request, but we prepare your code based on my pull request and after your code one commit changing the name of the "library". We prepare the code so as to facilitate the integration by Werner.

If Werner requires changes in my code, then I apply them and I also add that prepared code.
If Werner would not require any changes on mine, then we add a separate pull request with that prepared code.
User avatar
DevJohan
Posts: 41
Joined: Sun Jul 13, 2014 2:36 pm
Location: Stockholm, Sweden

Re: Solver/CGS architecture changes: Request for comment

Post by DevJohan »

I agree with what abdullah wrote about getting everything together, my code is somewhat on the fringe. I am currently pulling his code into my branch. The amount of conflicts are negligible and easily solved but the amount of new code is quite impressive.

From what I have seen so far I get the feeling there is a fair amount of cleanup and sorting out to be made in the freegcs code. There are some references in the code to thread page on this forum which I think should instead be references to a wiki-page or more specific post where the error functions and partials are explained.

I think flatGCS is a good name, it gets my vote. (do I get a vote :D)
Post Reply