Scope & changes to $special variables in "use<>" context #3881
Comments
|
Thanks for digging up the past discussions. Some of those predate my involvement so I'll try to read up on those. Using the "experimental" feature is not an option as this is a) global to the application b) defined by the user not the library author and c) not available in releases. I've outlined the options I'm seeing already, with basically those 2 allowing progress in development:
I'm open to other suggestions, except "just revert the bugfix". (note: you pinged the wrong user, sorry for using different nick names everywhere 😄) |
I approve. :) Scope discussions have the capacity to be enjoyable.
That's in fact one of the core things that was broken. They could NOT be dynamically redefined by the parent file. The call chain dynamic scope is ignored in the old behavior when it is overwritten by top level values in the lexical scope of the used file. Under the old behavior it was possible to do things like the is_undef trick at the top level to dynamically decide in the lexical scope of the used file whether or not to keep the old value based on the dynamic scope value being passed along to the function or module, but the actual process of this is that the lexical scope assignment dominates over the dynamic value from the call chain.
In other cases I'd be right beside you with that proposal. But because of the nature of the code involved with this change, it's not a very viable approach for this one. It was actually changed during a major clean-up of the context management system, across huge sections of the code, designed to make that code easier to work with for future development. It would be tragic to lose that progress that was in the way of other things (like enabling memory stable function literals). It's also not very viable to maintain two versions of such a large sweeping area of the codebase (where in this case the non-experimental version would have active memory leaks).
There's no problem with them being local to the used file. I think that part isn't in any disagreement. The problem is the surprise overrides at certain call boundaries. Let me demonstrate the core logic issue with this example: main.scad: used.scad: Bar1 and Bar2 have identical code, except for the $test having a 1 or a 2 in it for clarity. They call exactly the same module. Bar1 and Bar2 are called from exactly the same place in the main file. So you would expect the same code called from the same place and calling the same thing to do exactly the same thing. And it does in master. But, in 2021.01 this is the output: I think that's some very perplexing behavior. If someone posted this exact example as a bug report in an issue, I'd promptly say yes, fix it. To me the core question is how can we support the old behaviors identically? Well we can already implement them all the same way with functions and/or function literals, in a way fully supported by both 2021.01 and master. Imagine for a moment the hypothetical where the current behavior in master is how it always was. If someone right now proposed we supplement dynamic scope $special variables with top-level !magic variables which do not follow dynamic scope, but instead get processed every time a used file is entered and then are lexically available to all the functions and modules in that file, I'd have to think really hard about this proposal. I'm not sure if I could justify !magic variables where EVERY !magic variable is reprocessed at every external call of a function or module, when it's so easy to get the same functionality with functions in a way that's clear, explicit, in-place, and selective to only the ones needed in a particular called function or module. These are my thoughts about the general language design questions involved. |
|
Bear with me, I tend to need to pick things up and examine them from different perspectives. OK, I took that and turned it into a more common case with main#3881&used#3881.zip (disregard $fX - something I'm still playing with) With 2021.01.Included:Used:The default This is the beginner mode of library development, create and test your library, from the days prior to tabs in the editor. Then With 2021.7.29.Included:Used:So now they both have the same effect, all modules in the Anyway, onwards...ignore me for now. |
They could with the So this change doesn't really fix a bug or provide a means to get rid of the re-evaluation hit. It just breaks a common paradigm. I think the correct fix is to put them back in the search order in the same place where their neighbour file scope variables are searched. It make no sense that The only bug that needed fixing was file scope $variables beat dynamic scope. As long as dynamic scope beats file scope just as local variables in functions beat normal file scope variables then it all makes sense. |
I updated the testcase to evaluate this. main.scad (identical to above): used.scad: 2021.01 outputs: The master branch outputs: This seems right in master, as "$test = "from used.scad top level"; test=$test;" behaves properly for something which is evaluated once declaratively in a single-valued manner for the top-level scope, rather than dynamically at each call. It is a different $test value in the top-level evaluation because it is at a very different point (the top level) in the call chain process, so the variables pick up what is set right next to them. |
That is one interpretation. The problem is OpenSCAD has interpreted it differently up until now. Any lexical expression that depends on a $variable is dynamically re-evaluated when the $variable gets a new value. If I change main to: And used to : Then I get this with recent master and the last release: It shows that lexical value of A simple example is here: https://github.com/nophead/NopSCADlib/blob/52e9c1d7fd97555a03ea9c029887902d28aefae5/printed/cable_grommets.scad#L28
So clients of the library can set With the last release I get: But from master I get: This creates subtle bugs with my library. Everything uses the default values and ignores any file scope overrides, when used from another file. |
|
@rcolyer , you say:
My interpretation of what you are saying is that the old behaviour is incorrect, and if we had a chance to start over again, we would never have done it that way. So let's work on the assumption that the old behaviour was wrong and the new behaviour is correct. Even if that is the case, it doesn't mean that it is correct to change to the new behaviour. There is a body of code that relies on the old behaviour, and it is a non-trivial and error-prone exercise changing that code, see #3781 (comment) . A piece of software like OpenSCAD needs to ensure that new releases don't break old code, even if that involves maintaining "bug compatibility" (ie not fixing bugs because the bug is relied upon). Maintaining bug compatibility is a well established practice in software development, and although it sometimes leaves a bad taste in the mouth, it's just one of those aspects of software development that you have to accept. One of the classic examples is the Lotus 123 leap year bug - the Lotus developers incorrectly assumed 1900 was a leap year, which meant that the serial date number was out by one after March 1900, and many date functions relied on this value. So when Microsoft Excel came out they reproduced this bug, even though it could easily have been fixed. See https://docs.microsoft.com/en-us/office/troubleshoot/excel/wrongly-assumes-1900-is-leap-year
Or look at the steps Python took when moving from Python 2 to Python 3 so that existing code would not be broken. OpenSCAD is a piece of software with a large body of users. It is important that when those users migrate to the next version their designs are not broken. |
|
Your comments on compatibility are in general correct. However, taking the opposing side for a moment... Changing consumers to cope with an incompatible change is a one-time cost, while leaving a bug in place (or failing to implement an improvement) is a long-term cost (or the lack of a long-term gain). The cost associated with the incompatible change will only grow with time; however many cases are affected today, the number will only be larger tomorrow. One of the reasons why new systems occasionally replace old is that the old are hobbled by their legacy, while the new are not. The challenge is to find a way to improve while maintaining compatibility or, failing that, to improve while minimizing the cost of fixing the consumers to get past the incompatibility. I haven't tried to deeply understand how Nophead's library uses this behavior, but perhaps there are alternate behaviors that are similar enough while still allowing for improvement. Perhaps, for instance, the file-scope variables (both lexical and dynamic) might be calculated once, at "use" time, and those values preserved. Later, at call time, those dynamic values might be inserted in the call stack between the dynamic values from the "using" file and the call-scope values from the first call. Effectively, the call stack would be a stack of file-scope sets of variables, and then a stack of call-scope variables. The file-scope dynamic variables would still be visible to their file, which might be enough. The incompatibility would be that they would not override call-scope variables passed by the caller, but maybe that's close enough to the current behavior. Alternatively, perhaps "use" could be deprecated in favor of "include", with new features allowing one to achieve the same goals. There could be a special variable that says whether the current file is the top-level file; one could use that to control whether test/demo objects are created. Getting a private namespace for file-scope lexical variables is harder, but object literals would help, maybe, by allowing one to gather all file-scope variables under one name. (But: it would be hard for them to depend on one another.) Or perhaps something more ambitious, some kind of explicit namespace marking that would allow for a layer of lexical scoping with a mechanism for explicitly exporting names (modules, functions, or variables) as desired. Or perhaps "use" might be deprecated in favor of still another inclusion mechanism with better semantics. |
|
Having file scope dynamic variables that are set once means they are not really dynamic. If you capture their value in a static file scope variable it would be fixed at one value. Yes you could override the file scope dynamic one but that would be confusing unless you made accessing file scope dynamic variables illegal from file scope.
I think the only bug in the old system was file scope dynamic beats stack based dynamic. If that were fixed I don't think I would have any problems. Instead they are now just missing from the lookup. I think they should be looked up just the same as static file scope variables as the same level but only if there isn't a dynamic version on the call stack. Just like local variables in functions. They are found in preference to file scope ones with the same name. The separate issue is re-evaluating all the static variables at call time. I think that should be tackled by marking which ones depend on dynamic variables and only recalculating those if the dynamic variables they depend on change. In the vast majority of cases file scope variables don't depend on dynamic ones and even ones that do, the variable doesn't often change from one call to the next. So for every file scope variable and function maintain the set of dynamic values they depend on and the last value of those variables it was calculated with and only re-evaluate it of one has changed by being overriden. |
|
@jordanbrown0 , I don't disagree with anything you say.
Yes, sometimes compatibility needs to be broken, which is why I mentioned Python 2 vs Python 3. However the decision to break compatibility should be a considered one, looking at the benefits, but also who is affected and by how much, and how easy it is to work around the incompatibility. And as you say, you need to minimize the cost of fixing the consumers to get past the compatibility. Part of the problem here is that the incompatibility seems to be the result of a significant code refactoring and so is difficult to isolate. It does seem there are a number of suggestions on how to move forward, perhaps someone (maybe you @MichaelAtOz ) could enumerate these so they could be discussed. There are two things to be discussed:
I think it is important to discuss the desired solution without concern about how difficult it is to implement. That is nothing should be excluded because it is too difficult to implement. Once a solution is agreed, then is the time to look at what compromises, if any, are required to actually implement it. |
|
IMHO 1. is what I posted above and 2. is also outlined above. I think we can all agree that file scope $variables trumping stack based ones is a long standing bug. I worked around it with my A backwards compatible way to fix the bug is to just tweak the search order. The current master just ignores file scope $variables in used files and that breaks everything. Simply looking them up in the lexical scope if they are not found in the dynamic scope fixes everything I think. Fixing the massive speed hit from evaluating everything on every call is a separate issue because the current master still does that. It is an optimisation problem, not a reason to completely change the semantics. |
|
I'm having a problem wrapping my head around the notion of (a) file-level dynamic-scope variables being at the bottom of the call stack, and at the same time (b) file-scope assignments (of both lexical and dynamic variables) being evaluated on each call. I come up with two more-or-less simple mental models of how these variables interact. In both of them, the file-scope variables are all evaluated at the same time. In both of them, for both types of variables, the "most recent" assignment wins. In one of them, that time was "now, at the time we called into this file", and the other is "a long time ago". I think those both produce relatively simple and understandable results. (NB, however, that "simple and understandable" does not always mean "good".) The model that says "for dynamic variables, favor call stack over file-scope, and evaluate at call time" says that the evaluation is done "now", but the values of dynamic variables are as if they were created a long time ago. The idea that might produce any number other than 5 makes my head hurt. I understand, technically, how at call time we could assign 5 to $a, and then when we look up $a to assign to b we find a "newer" $a, but I think I would find that very confusing and unintuitive. For extra fun, consider and observe that b and d might not be equal, if the caller overrides $c. I think that file-level variables should effectively form a level of the call stack, both lexically and dynamically. We should be able to identify one point in the call stack where they are evaluated - and they should then behave exactly as if they are evaluated at that point in the call stack, as if there was a module definition/invocation that included them. That point might be at "use" time, or it might be at "call" time. Although I think that abstractly both models are equally consistent, I favor the "at use time" model. I don't think of the file level as a true level of the call stack. I think of it as part of the "background", stuff that in a conventional language happens in the compiler or, at the latest, at file-load time, before you start executing anything - and certainly not every time you call a function in the file. And of course the "at use time" model is always going to perform better, without needing any complex optimization to avoid pathological behavior. |
|
Performance tidbit: I measured the performance of using a constant, a lexical variable, and a function to retrieve a number. My file looks like where SOMETHING is either a Execution times were: So the function costs more than the variables, but not orders of magnitude more. |
|
In the following, what should the "stack" of values be, where the top-most (last) value in the stack is the one printed? 1.scad 2.scad Of course 4 comes last, but what about the other three? 1,2,3,4? 1,3,2,4? 3,1,2,4? In thinking about that, I prefer 1,3,2,4 or 3,1,2,4, but I'm not sure which. (1,2,3,4 requires evaluating used-file file-level assignments on every call.) My first thought was 1,3,2,4, that "used" files would layer in atop the files that "used" them, but in thinking about it more I'm not so sure. 1,3,2,4 would mean that a main file that says would be very different from one that says ... and that's a very bothersome thought. 3,1,2,4 also has the advantage that 2.scad has lots of control: it can supply a default (3), accept a default from the main file (1,2), and override that default (4). I guess that 1,3,2,4 can get the same control by having the "used" file do but... bleah. |
But that is exactly how it behaves now and has always done. The only difference with master is you can't see The way I got around this long standing bug was to not set And I include the file with this definition in every file in my library the project files that use it. So if Also with the last release and all versions I can remember before it going back to Mendel90 in 2011, I can set If I wanted to just change So it was an amazingly flexible mechanism to have global defaults in a library and be able to override them very selectively. |
Currently any function call into a module evaluates all the file scope variables of that module. My library has very few file scope variables internally, so calls into it are fairly quick, but as soon as I have a multi-file project calls between files explode in time because each file in the project has lots of list constants from the library, possible hundreds and fasteners, etc. Any call then re-evaluates all of them despite the fact they are constant and it gets to hundreds of thousands of evaluations and can take 2 minutes. |
|
To be clear: I don't think any amount of re-writing my library can get this flexible configuration scheme back. even if replace all my file scope constants the depend on dynamic variables with functions, the fact that file scope dynamic variables are no longer seen by functions means that I can't override things at file scope. I would have to do it on every call. |
|
There is also the issue that even if NopSCADlib could be re-written, then (virtually) all projects using that library would also have to be re-written. |
|
[ Jordan wrote: ]
[ Nophead wrote: ]
That that echo could produce something other than 5? I can't get any build of OpenSCAD to produce anything other than 5, because either:
|
Yes, and that needs to be fixed. (And the need to fix it is one of the biggest reasons for playing with the evaluation and scope rules.) But it's also not entirely relevant. A function call into a "used" file evaluates all file-scope variables in that file. But a function call from one point in that file to another does not; file-internal use of functions is cheap. |
|
The latter is true I think. When the file is used it will see But if you change it to this: bar.scad: Then The fact that file internal function use is cheap is not relevant because if I make all my global constants into functions they won't be internal.
It needs to be fixed by not re-evaluating variables that can never change, or haven't changed since the last call. That is nearly all of them. If it is fixed by changing the semantics then my library can not work, so I will have to stick on the last release or fork OpenSCAD and maintain a version with the current semantics. That seems like a lot of work though, so unless OpenSCAD came out with a killer new feature I would just stick with the last release for the command line to get correct STLs, etc and wait for the first snapshot that fixes the black window problem and use that for the GUI. |
|
For a real world example consider the following scenario: I want to design a big machine that has lots of tiny toolheads, so I decide to print the chunky parts of the machine with a 0.6mm nozzle and the tool heads with a 0.3mm nozzle. With the last release I can do this: In The toolhead is made from lots of little parts and they call into the library to make teardrop holes that are corrected for the staircase effect. When teardrops.scad is envolked from toolhead.scad it sees So the static file level variable in |
|
You bring up an interesting point. In the "evaluated once" model of file-level variables, simple A>B>C stacks will evaluate the file-level variables once, and A, B, and C will each have their own particular sets of values. But that's not really right, because inclusion is a DAG, not a stack. You can have A>B, A>C, and B>C, with C included by both A and B. In such a case, you need a set of values for each path, not for each file per se. I think much (but perhaps not all) of your desire would be met by evaluating the file-level variables once per "use" (and thus twice for C, for the case above), and then considering the call stack to be [ using, used, ..., calling, called, ... ]. Thus, if we have the "use" graph above, and a module Alpha in A calls a module Charlie in C, Charlie would see the last value set by: [ file-A, file-C, module-Alpha, module-Charlie ]. If Alpha in A calls Bravo in B, and that calls Charlie in C, Charlie would see the last value set by [ file-A, file-B, file-C, module-Alpha, module-Bravo, module-Charlie ]. File A could set a value that would be used across the project, unless overridden by B, C, or one of the modules. Note that a particular file could use the "is_undef" trick to supply a value only if its "parent" file didn't supply one. Note that module settings always override file settings, so when Alpha calls Bravo calls Charlie, if Alpha sets a $ variable its value would override any file-level values anywhere in the picture. What that doesn't allow for is file-level variables (whether lexical or dynamic) that are set based on module-supplied values. |
|
A core goal needs to be having a language which is clear and teachable, following consistent rules. I'm pretty sure that the majority of the program's users come in with somewhere from modest through no programming background. Therefore we should sustain simple rules to give them for where to look for where a value came from. Extra rules for dynamic values slipping in from side-locations potentially very far away (e.g., in an included file of an included file inside of a used file?!) can make it very difficult to debug any code, because you literally have to check an entire tree of included files just to be sure there are no side-sections of code that conditionally change a dynamic value. Finding where a variable's value was set should not resemble the C++ rules for argument dependent lookup. :) The lexical and dynamic rules in standard form are pretty straightforward to explain, and pretty straightforward to track when debugging, which is why they persisted across so many languages. Absolutely everything needing a dynamic default can be done with a function. I stipulate there is a very minor performance overhead to a function call over a variable setting at the top level, but there are three very important things to consider: One, this overhead is so small that you have to do quite a lot of repetitions just to measure it, whereas the typical usage does not have this property at all. Two, the overhead is less than a factor of two, which means as soon as we fix the bug from repeated evaluations, then in most code that will make up for the function call performance overhead several times over (this is a fundamental gain, from doing less repetitive unnecessary work). Three, the current function call performance head is not intrinsic to function calls. The old assignments and the function calls are doing the same fundamental work internally, so there should certainly be an opportunity to optimize the function call to get the same speed, which means we should not design semantics around a small arbitrary performance difference in the current implementation. A benefit of the function calls is that it is clear what they are doing. There is no magic appearance of a dynamic change. You call a function specifically to evaluate some value dynamically, and thus the result you get from it has exactly the behavior of the semantics on the label, and there are no surprises. This makes them easy to understand, and straightforward to teach and explain. |
|
Well I would have to rewrite everything I have written in OpenSCAD for the last 10 years or so and can no longer set And any file scope capture of a dynamic variable would need to be an error because its value would be fixed, totally different from the current behaviour. So to avoid odd bugs and force everybody to rewrite their code it would need to be at least a warning. In @jordanbrown0's example: If you pass a new value of So basically you are making the language less powerful to make it easy to teach? |
|
@rcolyer , you haven't answered my previous comment addressed to you #3881 (comment) In particular
What is your view on addressing the fact the this change has broken one of the OpenSCAD libraries, and all projects that depend on that library? My view is that a breaking change can be made, but it has to be made in a considered way, weighing the costs and benefits, and also done in a way that allows a managed migration to the change (cf the change from Python 2 to Python 3). |
If you want to change dynamically reactive default behavior, you don't edit $layer_height in a used module, you edit layer_height(). This does not break any ability, it just requires adjusting the way you were thinking about how to do this. I don't think so much hyperbole is needed about this. I edited the two variables which you indicated were the big issue, updated 115 lines, and it took about 30 minutes for the changes since it was very formulaic. The amount of effort put into the issue reports about this has easily been more than 10x the effort to update the library to use standard dynamic scope.
There is a ton of utility to dynamic variables, and they don't need to be thrown out. Just, their utility is dynamic, not mixed dynamic/lexical. Their utility actually increases when they work cleanly, just not for the specific (and apparently uncommon) pattern you were using. That pattern is a functional pattern, and should be done with functions, getting the same result.
Using a dynamic variable in the top-level of a file is not an error, and should absolutely not have a warning. This is a well-defined process, and is extremely important to the behavior of a main file. The problem your library experienced was only a specific issue with attempting to get active dynamic behavior from the top-level of a used file. Additionally, using a file should definitely not introduce new warnings based on parts of the code not being used for a call, when the manual and other learning material we give people have already described for a long time the current behavior of the master branch.
b is a lexical scope declarative variable that should have one constant value. $a is a dynamically scoped variable that should update based on the call chain. The example quoted right there is actually an example of a hazardous bug from the old behavior, because the caller sets $a and the value is completely overwritten, without caller control, wiped out like it's a non-dynamic constant by something outside of the call chain. If $a were used directly in foo() this would be very bad. b, however, should stay constant, because it's not dynamic. The person editing foo should not have to inspect how the constant lexical value of b was obtained in the outer scope to determine that it stays constant.
It's not one drop less powerful. I've demonstrated how to do every dynamic behavior with ease. It's more clear, more debuggable, and more teachable, because lexical constants stay constant (and don't go surprise-dynamic), dynamics stay dynamic (and don't go surprise-constant), and active generation of dynamic values in a standardized manner is done by active function calls instead of by implicit function-like behavior that is not labeled as a function. Everything can still be done, just everything actually does what it says on the label. This is a very good thing.
My view is that it required minor changes to one library, and I showed that it is easy to fix that library, even providing a PR for the part that was highlighted as the biggest issue. Notably, the change I demonstrated in the PR should not require changing projects that depend on that library via use<>. The dynamic variables from the caller projects are still picked up in the same way during the function calls, so it's only an internal change in how behavior is made dynamic.
Yes. And I agreed with the changes from Python 2 to Python 3 as well, even though this required much more massive code updates (some of which I had to do). In weighing them, there are huge costs to the old use<> behavior in an accidental potentially-massive performance overhead and in the ability to read, reason about, and debug code, as behavior can sneak in as dynamic value changes from far away places, meaning even a tiny function or tiny module might actually have globally impacted dynamic behavior unless you check all over the place. Here this is much milder than Py2 to Py3, because frankly the steps to take for "managing" this migration would consist of clearly documenting in the manual and instructions how the variables behave and should be utilized and giving a stretch of time to adapt to this. And it turns out we already did that, as the way master behaves now is the straightforward way the manual and instructions said the variables should and would behave for 8+ years. (Which is why I have been calling it a bug fix.) The lexical/dynamic/functional way I'm describing to use worked all along, aligns with the manual, and was/is available to libraries. Honestly, there was just a disconnect between the way that particular library used them and the descriptions that one would use to manage and migrate such a change, and so the library should be updated. I've seen no indication that this was a widespread issue, and I haven't seen that rely-on-reprocessing usage pattern elsewhere. I think most users followed the guidance in the manual and other instructions for designing their code. I tried to help with this particular library, by both explaining and submitting a library PR with a chunk of the work to do this update. It should really not be very difficult for someone more familiar with the library to identify the remaining variables that need identical changes and slipping those in. |
|
@rcolyer , you state:
@nophead , is a smart guy, and I'm sure he has considered your proposal. If he says it doesn't work, then I am inclined to believe him.
The problem is is that is not how projects depend on the NopSCADlib library - projecst both You are a smart guy too, but you are not familiar with the NopSCADlib library, and have not used it for a large project. The fact of the matter is the library has been broken, as have the projects that depend on it. As I said, we need a managed way forward. What that means is something that can be done on a timescale dictated by the users, not a forced immediate change when a user upgrades to a new release. One example of this would be @jordanbrown0 's suggestion that |
|
@rcolyer , And I can no longer set $ variables in a used file to affect it and all the files it uses. I would have to set it in all the functions and modules of that file that get called externally. So when used and called the correct values are enforced. E.g. in bob_main.scad I set So now I would have to write a lot more code to get the same functionality as before, so in my mind the language is less powerful. |
You misunderstand my point entirely. What I am saying is the uses of dynamic variables need to be reevaluated. Either OpenSCAD does it for me or I have to do do it myself with functions calling functions calling functions all the way up to the outer dimensions of my objects. But on entry to a module only the dynamic expressions need re-evaluating and only if they are used and only if the dynamic variables have changed since the last call into the module. So if that is fixed the overhead would be tiny. Files full of maths functions probably don't use any dynamic variables at all, so they would never need to be recalculated. And in the vast majority of projects dynamic variables might only have one value and only a small number of expressions use them. But those that do must be recalculated, when they change and are referenced. |
|
I don't think that there's any way to bring these two viewpoints together. That leaves a couple of possibilities: (A) and (B) are unappealing. (D) requires a developer who is intimately familiar with the expression evaluator infrastructure, and carries substantial risk. Is there any developer interested in spending that effort? I think that leaves (C), which is still non-trivial development but probably doesn't involve a massive overhaul, just setting up options as to when the assignments are evaluated and how the call stack is built. It would also require figuring out what the semantics of the new style should be. |
|
It is still re-evaluating. Results. |
|
As you can see, |
|
Yes, I know that it's still doing that every-call-into-the-file evaluation. I'm actually puzzled by the current-snapshot behavior, from a black-box perspective, because it changed the semantics without addressing that issue. But that doesn't really matter, because that issue, the "evaluating file-level assignments on calls into a file" issue, is one that several people are interested in and should be assumed to be on the horizon - and (absent the major overhaul in (D) above) it will require changes much like this one, that will break nophead's use cases. If we abandon that issue, if we decide that we're not going to fix the performance hit, then (without looking at the actual implementation) I expect that restoring the old semantics would be practical. But if we address that issue (without the overhaul in (D)), nophead's library will break. |
Yeah, one step at a time. I think fixing that performance issue is one of the important steps before the next release. I was actually thinking about starting to look at that issue closer tonight, but this issue keeps chiming away with notifications. :)
Well, I think a keyword is the more OpenSCAD type of solution to it. First, let me propose a keyword possibility to declare a context that will be fully evaluated before entering called modules: The behavior of the above example would be that "use_context" is evaluated before entering the modules, so all the dynamically scoped variables are set as desired. In this tiny example, it looks great, and solves the first problem. There is one critical issue with it that we can solve, which is that in large projects it leaves potentially serious debugging confusion, because if ever you are confused about how a dynamically scoped value is changing, you have to suddenly search every included file in the used file to see if there is a use_context statement hiding in it. (For example, it can come 3 include layers deep from someone ELSE's library that you happen to have included instead of used.) That's a confusion risk that we should avoid, and that we can avoid easily, by simply making sure the keyword is applied near the modules that will use that specific context. So basically, we want the use_context keyword right next to the module, so that it's clear they will use that context. Here's an example of how to do that: This already works in every version of OpenSCAD, so please feel free to use it and get the desired behavior. |
|
I really don't understand why the language has to change to fix the re-evaluation issue. If you do If you do It isn't rocket science but you do need to mark functions that depend on dynamic variables as well. The suggestion above is ugly because instead of doing If dynamic expressions are only evaluated when they need to be in C++ that is more efficient than simulating them in OpenSCAD userland. |
Looks that way.
It hides complexity by looking after the mechanics under the hood. That's why we automate things. A key aspect of OpenSCAD that I try to maintain is its simplicity. I turns out that its current form creates performance issues with more expansive implementations. An additional option to add to Jordan's options list: (E) [possibly a variant of (C) depending on how you read it] *Later if resources are available, consider something like what nophead just proposed or build a dependency tree to only re-evaluate variables with a dynamic dependency. If there is recognition that something is needed for backwards compatibility, e.g. a pragma, then I don't see debate in circles is needed any further. It seem it is ideological. |
|
The disadvantage of creating a new way of loading modules or adding pragmas is that anybody wanting improver performance would need to use it and then their code needs the latest OpenSCAD. Keeping the language as it has always been and optimising the reevaluation means no old code will break, it will just get much faster and still be compatible with old versions of OpenSCAD and all the examples and books, etc. |
|
Also simply marking file scope expressions static or dynamic and only re-evaluating the dynamic ones on every call will give a massive speed increase and anybody who wants to avoid revaluation can simply avoid dynamic expressions in their used files. Keeping track of which variables are used and if they have changed is more effort and probably only gives marginal gains, except in unusual cases. For example drilling hundreds of holes using a circle in a used file that depends on $fn set at file scope. |
|
Michael writes:
My intent was that that was covered under (C). Either the caller would say "bring this file in with the new semantics", or the callee would say "when you bring this file in, use the new semantics". Ryan writes:
It will be possible to create spaghetti with any scheme. Nophead writes:
I haven't looked at the expression processing in OpenSCAD, but I've implemented one or two expression parsers/executors, and looked at one or two more. That change would have been approximately "rewrite the expression parser from zero". |
|
The parsing is done much earlier by code that is automatically generated. This is at the evaluation stage. As for being difficult to debug large projects, I probably write some of the largest OpenScad projects and never have any problem understanding or debugging the dynamic variables. They just worked until they were broken. |
|
And if the parsing stage didn't record the dependency data, the evaluation stage doesn't have the information it would need to know how to optimize. But, critically, nobody is jumping up and saying "oh, that's easy, I'll do it!". |
|
Well then it should be left as it was. Working but not optimum until somebody has the will to optimise it. Not its too difficult, so we will simplify the language and break the common usage of $variables. |
|
The evaluation stage knows when it is looking up a special variable as it has to do it differently, so I don't imagine it would be hard to record that an expression has dynamic terms and is then itself dynamic. Not trivial but not impractical. |
|
I have been dreading the email.* For others here, there was some offline debate on this after the last post here. The tail of that was: & Where I made bold commitments to follow up. *So I have been expecting the email saying, well with all this activity we're working toward a release, we need to sort out this. I have to apologise. I have not made any progress on this (or other matters) since. So, while this post may reignite this seemingly controversial issue, I need to do so to allow the wider project to evolve. Other than what happens in the back of your head while making other plans, I have not though much on this matter. So, sorry, but this goes on without my further two bobs worth. [I can cope with the relatively minor admin items, but am hundreds of emails behind. So please highlight anything along those lines you think needs my attention, prominently e.g. "READ THIS ONE"] |
|
After some discussion on the mailing list, I was encouraged to add a comment to this issue. Apparently, this change breaks a lot of my existing code. I discovered this when I attempted to use OpenSCAD version 2023.04.04 (git 102118f). The issue is easy to demonstrate. Given the file "B.scad" that contains a single line: Given a file "untitled.scad", that contains: OpenSCAD 2021.01 has no problems when previewing untitled.scad. The Dev Snapshot gives me these errors: The question came up of as to why I was "use"-ing a file that contains an include. I rely on “use” quite a bit. I design assemblies that house electronics, especially printed circuit boards. I design models (vitamins) of the circuit boards, their standoffs, mounting holes, etc. I use a standard template for modeling these circuit boards because, well, standards are good. I can develop a new circuit board model quickly and it’s easy to come back later and understand an old model. Each printed circuit model uses a standard set of variables, such as: PCboardX …and others that are defined once at the top of the file and can then be used freely in the various modules in the model. Prefixing these variables with a part name would be a whole lot of unnecessary typing. Since I know that I am going to “use” this model, I don’t need to worry about name collisions. The assembly should not need to know these dimensions and pass them as arguments. All of this works great in 2021.01 |
|
I have a bit of a headache at the moment, so maybe I'm not thinking this through well. However, several ideas are bouncing off of each other in the back of my head between this Issue, and the Object/Geometry-as-Data/Module-literals proposal in PR #4478. Given the concept of Objects and Module Literals, there suddenly become a couple Very Useful Things™ that become relatively simpler to implement. Things like NameSpaces and Object Oriented Programming. Regarding Object Oriented Programming, yes, OO does have uses, even in the immutable hell that is Functional Programming. See OCaml. This only needs implementation of a More relevant to this Issue is the idea of namespaces. If a functional foo.scad: MAX_SIZE = [42,36];
module weird_shape(size) {
// make geometry here
}
function weird_shape_info(size) =
// Calculate something here.
;bar.scad: foo = import<foo.scad>;
echo(info = foo.weird_shape_info(foo.MAX_SIZE));
foo.weird_shape(foo.MAX_SIZE);For extra efficiency, cache the Object when a scadfile is first imported, then just return that whenever something tries to import it again. For extra credit, make a special |
|
@revarbat : indeed, objects and a hypothetical new import mechanism provide an opportunity to improve namespace behavior and perhaps clean up unfortunate legacy. However, I don't think that's really the topic here - behavior that people were depending on changed incompatibly. |
|
Consider the following example: // library.scad
module foo() { /* something that depends on $a */ }
module bar() { /* another thing that depends on $a */ }
// library2.scad
use <library.scad>;
$a = 2;
module foo1() { /* something that depends on foo() */ }
module bar1() { /* something that depends on bar() */ }
// user.scad
use <library2.scad>;
foo();How would you convert this pattern according to the behavior in latest master? You need to set $a in every module/function in I think the dependency graph approach is fine, it basically solves the performance issue of re-evaluation. I am not too concerned about the issue of side effects, we can just say that there is no guarantee on evaluation order and the number of times these file-scope variables are evaluated, any behavior depending on that is undefined. This is not worse than the current state of affairs: we are now re-evaluating the variables on every use, which is worse. Fixing the re-evaluation problem with the current semantics will change the side effects anyway. The only problem is this becomes harder to explain, but if we consider the order of evaluation undefined, it is not really that hard: just treat it as if we re-evaluate those file scope variables whenever you use them, caching is merely an implementation detail. For implementation, I am happy to do the refactoring, but for now I think we can put the variables back in the search (bad performance, but not worse than what we have now). This will probably be a major refactoring, so this will take some time after reaching a consensus. Btw, the current behavior is confusing and broken due to the re-evaluation issue, we need to fix it anyway: // foo.scad
a = $a;
module foo() {
echo("foo");
echo(a);
}
// bar.scad
use <foo.scad>;
a = $a;
module bar() {
$a = 20;
echo("bar");
echo(a);
foo();
}
// foobar.scad
use <foo.scad>;
foo();
use <bar.scad>;
bar();Output: |
|
Note: This will work. module wrapper() {
$a = 0;
children();
}
module foo1() {
wrapper() {
foo();
}
}
module bar1() {
wrapper() {
bar();
}
}
foo1();
bar1();I forgot openscad |
|
Yes you can add a wrapper to all modules that get called externally to replace a single $variable assignment at the top of the file but what about functions? You would need to add a let($a = 0) to any that reference $a. Very ugly. |
|
yes, and apart from that there is also performance issue, as you must re-evaluate each of them every call |
|
The performance issue is a non-issue if you want to re-evaluate each of them every call. |
|
No? if they are in the file scope, the implementation can cache the assignments and only re-evaluate when the dependency changed. It is not easy to do, but possible. |
|
This has been an argument for keeping this bug but in two years the re-evaluation problem still exists. Also you can still copy a $variable to a normal variable in file scope and then access it from a module or function. So it still needs to re-evaluate the file scope expressions unless you make $varaibles illegal at file scope. |
Sorry, this took longer than I anticipated. It has threads from many places.
I've raised a new issue, as #3781 was getting large, and I want to try keeping the focus concentrated.
Firstly I wanted to confirm my understanding of how OpenSCAD WAS meant to operate prior to recent changes.
Everything has a historical context.
History.
We all know that there is no formal definition of the OpenSCAD language.
I had been using OpenSCAD (from 2011.something) for a while and started to have questions of how it was meant to work. Whether things I observed were bugs or undocumented features etc.
At that time the wiki was fairly basic.
So I joined GitHub (c 2012) to report bugs, which often generated debate on OpenSCAD behaviour.
When clarity arose I tried to improve the wiki with the new details.
I note that as a reminder that the wiki is NOT strictly the definition of the language.
'Variables' & scope were not documented anywhere, except maybe in @kintel's @donbright's heads at the time. Read this if you want to know how it was back then.
I hadn't joined the Forum at that time, but was an avid lurker.
Things like Variable-bug, where the above @kintel quote came from, had something familiar to recent discussions:
That was at a time where you could not assign in higher scopes, you required the
assign()module,which basically created a child module scope, there were root variables and
module(),function()&assign()scopes.The first meaty one is Can't override include variables (March 2013)
& (at that time)
& (at that time)
Then came Scope refactoring & Variable assignments in local blocks, (May 2013) this introduced File-Scope and Local-Scope, allowing assignments at local scope. Note
use<>predates this, they have the hidden File-Scope including $variables.The above Variable-bug lead to the next issue, brining
-D var=valinto the equation, Regression: Variable override issue, (Jun 2013) a range of topics.@kintel
Note the 'etc.' includes the hidden
use<>File-Scope.@kintel
I'll skip forward for brevity to a comments in Feature request: Enums / Enumerations
(comments Apr 2016)
@kintel
[Note: primarily to not pollute the global scope]
So basically the
use<>special hidden File-Scope has always included top level $special variables.They have been actively utilised there as defaults (particularly the $fX's) and been able to be dynamically redefined by the parent file of the
use<>.This has been my understanding too.
I haven't specifically gone looking, but I have seen plenty of cases in the wild.
This all comes down to having to do workarounds to get needed functionality for complex projects.
The new behaviour introduced recently is a breaking change. It is not fixing a historical bug.
Yes, from a purist view it can be considered fixing a flaw, but OpenSCAD has always been a bit impure when compared to formal languages.
Nophead obviously has a lot invested in this, his framework and libraries are a phenomenal achievement working without formal mechanisms usually available.
BUT this is not just nophead, as I said it is in the wild, there will be plenty of discreet libraries that get broken if it went forward as it.
Where too? NOTING THIS IS IN CONTEXT OF
use<>- a particularly OpenSCAD-esq context.For a start, I would propose taking some heat out of this situation.
Initially by saying the dynamic scope change does not need to be introduced right now.
I know there are other drivers for fixing other issues, but there is time to consider that.
I have yet to go over the last few days of counter-discusions, and impact on the performance problems. I'll get to that (later). Particularly the re-evaluation aspects. And performance is important, but perfection takes time.
Perhaps one or both of the below may be an interim step.
a. Make the change experimental, so noting it is not an accepted final design solution,
b. Nophead suggested at one stage, see below*, a backwards compatible direction.
Thus a. could allow testing impact, without breaking frameworks, while letting other development changes accessible.
The real solution is to progress to the various namespace-esq library versioning and packaging solutions (discussed off & on for a while), then depricate
use<>, that will mean staging 'fixes' to scope.So that is the ravings of this particular idiot. I'll go and read all the latest conjecture...
I hope this hasn't upset @tp too much.
:)Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.
The text was updated successfully, but these errors were encountered: