sketcher : invalid constraint cannot be selected or deleted

About the development of the Part Design module/workbench. PLEASE DO NOT POST HELP REQUESTS HERE!
Forum rules
Be nice to others! Respect the FreeCAD code of conduct!
chrisb
Veteran
Posts: 53945
Joined: Tue Mar 17, 2015 9:14 am

Re: sketcher : invalid constraint cannot be selected or deleted

Post by chrisb »

adrianinsaval wrote: Thu Apr 27, 2023 4:15 pm You are right and realthunder himself was not an expert either when he started this journey, what I don't like is the oversimplification of both the problem and the solution, treating an inherently complex problem as if it was just due to an oversight.
I fully agree!
A Sketcher Lecture with in-depth information is available in English, auf Deutsch, en français, en español.
User avatar
freman
Veteran
Posts: 2203
Joined: Tue Nov 27, 2018 10:30 pm

Re: sketcher : invalid constraint cannot be selected or deleted

Post by freman »

adrianinsaval wrote: Thu Apr 27, 2023 1:02 pm "Just use linked lists" is not a solution to TNP, people often come here and read some short description of some programming concept and think "well looks like I found a solution to TNP! Why is it that all these programmers with years of experience working on FreeCAD haven't thought of this before?"

But obviously solving TNP problem is much more nuanced than that... There's people working on it already, it's been going on for years now with a lot of thought and research into it, leave it to the experts unless you actually know what you are talking about and can code.

Likewise the constraint renumbering problem is also more nuanced and I have my doubts that linked list are of any use here, after all when using expressions we are not sequentially accessing members of the list so the pointer to the next member is useless, we want to access one specific object via an identifier, linked lists don't magically solve the identifier problem nor are they necessary to solve them. I could of course be wrong since I'm also an amateur programmer
First of all , don't get shirty, especially on other people's behalf. I've seen "experts" make some silly errors before like the memory leak which was eating up 4GB of RAM because python lists were not being used properly. I can code , I found it and fixed it. Everyone makes errors, that happens. So even experts need humility and proxy defenders need proxy humility.

I don't doubt that the problem is more nuanced, once there is a body of code built around something it gets complicated, even if the original error was simple.

"we are not sequentially accessing members of the list ". I never said the problem is sequentially accessing them, it is deleting them which, using dynamic arrays means automatic renumbering. They are clearly being referenced by number because we can see references disappearing or referencing different data when a constraint is delete. Linked lists would prevent that happening, since only the deleted element would be affected, not all elements later in the array.

At that point you should consider taking your own advice ... "unless you actually know what you are talking about ".

It won't be as simple as just changing the object type, because the code will need to be changed now it is written for dynamic arrays. The suggestion was intended to be of possible use. Not in invitation for a flame war.

The sketcher and the solver are quite complex code, so changing the object type at this stage may involve a lot of code changes, although that would probable not be difficult, there may be quite a lot.

From my superficial knowledge of RT's work on fixing the topo instabilities, I think it involved using lists to keep track of the ephemeral geometry references. Perhaps linked lists may have been better from the outset. Someone closer to the code could assess that.
User avatar
adrianinsaval
Veteran
Posts: 5541
Joined: Thu Apr 05, 2018 5:15 pm

Re: sketcher : invalid constraint cannot be selected or deleted

Post by adrianinsaval »

Indeed on the constraint issue I admitted myself to not be sure, it's just the toponaming issue that I'm certain this is not the problem. I have no interest in flame wars either so I'll apologize for my behavior, I think you are right and it wasn't the most appropriate, have some understanding towards me though, I'm a little tired of every guru that claims to know exactly how the toponaming problem can be solved and how the programmers where stupid for having used unstable references, one guy once even asked for special treatment to his bug reports in exchange for his "brilliant" ideas :roll:
From my superficial knowledge of RT's work on fixing the topo instabilities, I think it involved using lists to keep track of the ephemeral geometry references. Perhaps linked lists may have been better from the outset. Someone closer to the code could assess that.
This is a complex fundamental problem that every CAD faces, it does not come from insertion or deletion of elements in a list as is the case for the constraints so I doubt linked lists offer much benefit there. The lists are unavoidably regenerated from scratch when a shape is recomputed and the task is to try and remap the references from the old list to the new list. For this it is required to know the modelling operation involved in the creation of the elements.

Let's continue discussing on the subject of the constraints references only then, with hindsight indeed it might have been better to use a different approach from the get go, but I do not blame those who made this for not having predicted this situations
freman wrote: Thu Apr 27, 2023 6:52 pm "we are not sequentially accessing members of the list ". I never said the problem is sequentially accessing them, it is deleting them which, using dynamic arrays means automatic renumbering. They are clearly being referenced by number because we can see references disappearing or referencing different data when a constraint is delete. Linked lists would prevent that happening, since only the deleted element would be affected, not all elements later in the array.
I can see the appeal for the constraint list, assuming the constraints will still be referenced by index in expressions but these would be immutable, I think this is essentially what zolko asked for in the beginning (which I liked). I think abdullah wanted to support simple python lists for scripting though.
Now, can the constraint list really be considered a simple list? Yes and no, it's possible to reference constraints by name not just by index, do we actually need to change that much to get the functionality we want?

There are at least two other solutions for this that don't require changing types, either we add some other persistent immutable identifier (this is chrisb's proposal in the other thread) and use these in expressions, or we automatically assign names on creation to these constraints (my proposal). Obviously I'm biased towards the later, which I consider easier to implement and more flexible for the user as he can choose to remove a constraint and then reassign it's name to another.
User avatar
freman
Veteran
Posts: 2203
Joined: Tue Nov 27, 2018 10:30 pm

Re: sketcher : invalid constraint cannot be selected or deleted

Post by freman »

apology gratefully received. Thanks for the positive response and the extra info on the topo problem. You are correct , it should be kept separate from the constraints issue.
Post Reply