loopspace / spath3

TikZ/PGF package for manipulating soft paths, includes the knots and calligraphy TikZ libraries.
16 stars 2 forks source link

! LaTeX3 Error: Inconsistent local/global assignment #6

Closed pablgonz closed 3 years ago

pablgonz commented 4 years ago

Hello, using this example:

\documentclass{article}
\RequirePackage[enable-debug]{expl3}
\ExplSyntaxOn
\debug_on:n { check-declarations , deprecation }
\ExplSyntaxOff
\usepackage{tikz}
\usetikzlibrary{knots}
\begin{document}
\begin{tikzpicture}
\begin{knot}[
  draft mode=strands
]
\strand[red,thick] (0,0) .. controls +(1,0) and +(-1,0) .. (2,1) .. controls +(1,0) and +(-1,0) .. (4,0);
\strand[blue,thick] (0,1) .. controls +(1,0) and +(-1,0) .. (2,0) .. controls +(1,0) and +(-1,0) .. (4,1);
\end{knot}
\end{tikzpicture}
\end{document}

You get

! LaTeX3 Error: Inconsistent local/global assignment

For immediate help type H <return>.
 ...                                              

l.38 ...th_moveto_tl {\pgfsyssoftpath@movetotoken}

( I'm using TeXLive 2019 updated) Saludos

loopspace commented 4 years ago

Hmm, I'm going to have to think about this one. I can fix some of those easily, but others would need changing some of the internal ways I implemented stuff. And while I can see now that I made bad choices originally, fixing them will take time.

pablgonz commented 4 years ago

@loopspace Grateful for your work, I am sorry to give you headaches with this :( , the package as well as this one works, but, since you use expl3 and this one is in constant evolution you must make adjustments (only in order to do things the right way). I won't add the comments I put in hobby but I think they would be great here too. Saludos

loopspace commented 3 years ago

I don't know if you're still monitoring this, but if so then I'd appreciate a bit of advice.

The underlying problem is that I want to define data structures on top of the programming layer in LaTeX3 (in this example it is an spath object, but the idea is more general). I come from an object-oriented background, so my concept is to build a structure on top of an associated list (which in L3 terms is a prop) and construct methods and attributes on top of it. All interaction should go through the constructed methods and no-one other than me should ever interact with the underlying prop. So the name of the prop is hidden from the user.

I want to be able to use this structure both locally and globally. Looking at interface3 then props are always created globally. But I can't use both \prop_gput and \prop_put on the same prop. So I can't have a global structure which I make a local change to. This is a problem.

Here's an example from this package. I've now defined a routine that will shorten a path along its length. I might wish to render both the original path and its shortened version, and the order might differ as to which is first. To define the path I need to work globally since it will be created inside a group. Then to render a modified version I might change it locally so that the modification only persists for that rendering.

If I force everything to be either local or global then I have to make sure that the user keeps making clones of the object when they want to switch it from local to global or vice versa. It isn't often clear that something needs to be global as the grouping might be hidden. So this feels like putting an extra burden on the user to enforce some programming paradigm that has no value for them.

An alternative would be to make my objects immutable and to programmatically spawn off new copies every time a change is made. But this seems to be a case of a cure being worse than a disease as then there will be vast numbers of these data structures in memory which are no longer being used.

In short, in the following program then every single one throws an error and I don't see a way to avoid this while still keeping the flexibility that I want in my code.

\documentclass{article}

\usepackage[enable-debug]{expl3}

\ExplSyntaxOn
\debug_on:n { check-declarations , deprecation }

\group_begin:
\prop_new:N \g__my_prop
\prop_gput:Nnn \g__my_prop {stuff} {random}
\prop_put:Nnn \g__my_prop {stuff} {determined}

\prop_new:N \l__my_prop
\prop_gput:Nnn \l__my_prop {stuff} {random}
\prop_put:Nnn \l__my_prop {stuff} {determined}

\prop_new:N \x__my_prop
\prop_gput:Nnn \x__my_prop {stuff} {random}
\prop_put:Nnn \x__my_prop {stuff} {determined}

\group_end:

\prop_show:N \g__my_prop
\prop_show:N \l__my_prop
\prop_show:N \x__my_prop

\ExplSyntaxOff

\begin{document}
\end{document}

I realise that I'm fighting against the LaTeX3 programming paradigm a bit, but I'm not seeing a benefit in conforming to it here that will make up for the considerable hassle involved.

PhelypeOleinik commented 3 years ago

The problem is not really to use both global and local assignments, but to switch back-and-forth between them within a group.

Taking the \a and \g example from the TeXbook (\a is a local assignment, and \g is global), this will eventually exhaust the save stack, if there are too many instances of \a\g inside the group:

\def\a{\advance\day by 1 }
\def\g{\global\a}
\day=1

{\a\g\a\g ... <many many more> ... \a\g\a}
\bye

whereas if within a particular group you always use either only local or only global assignments, you will be fine:

\a\g\a\g % at group level 0 it is fine to mix them
% (but your code will be in a `tikzpicture`, so you probably don't have this option)

{\a\a\a\a ... <many many more> ... \a\a\a} % in a group only local
{\g\g\g\g ... <many many more> ... \g\g\g} % in another only global
\bye

What really cannot* happen is that in a group level > 0 you have a local, then a global assignment. The group level > 0 will cause TeX to store the local assignment in the save stack, then the global assignment will require yet another save stack entry (though once the group with the intermixed local and global assignments ends, TeX will clean up the save stack entries removing the items that were added in that particular group). It is actually kind of hard (you need thousands of assignments) to exhaust the save stack if you are somewhat careful.

(not sure how much of that you knew, so apologies if I'm taking sand to the beach :)

All that said, in terms of actually exhausting TeX's save stack, if your code is structured more or less like

\prop_new:N \g__my_prop

% global assignments in the “top” level of the code (not necessarily \currentgrouplevel=0)
\prop_gput:Nnn \x__my_prop {stuff} {random}

\group_begin:
  % all local assignments within a group
  \prop_put:Nnn \x__my_prop {stuff} {determined}
  \prop_put:Nnn \x__my_prop {stuff} {determined}
\group_end:

% back to global
\prop_gput:Nnn \x__my_prop {stuff} {random}

then you shouldn't have problems with the save stack.

But you will with expl3's debugging still :)

Changing the prefix doesn't help because if the prefix isn't known, l3debug will “remember” your variable, and ensure that you used the same type of assignment. However I think it's better if you use a prefix other than l/g/c to identify these variables that use mixed assignments, so one can easily spot them in the code. I think x is fine.

You have two options here:

  1. \debug_suspend: just before some code that you know that l3debug will not like, and \debug_resume: right after (\debug_suspend: and \debug_resume: can be nested, and they are defined even if l3debug isn't loaded, so you can use them right away). This is the preferable option.
  2. You can make a private copy of \prop_(g)set:Nnn (if l3debug is not loaded or \__debug_prop_(g)set:Nnn if it is), then your private copy will not have the debug patches and won't error. But this solution is not really advisable. Use it only if you are in performance-critical code and end up with too many \debug_suspend:.

Hope this helps. Feel free to ping me if I can help with anything.

pablgonz commented 3 years ago

@loopspace Great that you are working on this :)

@PhelypeOleinik Thanks for contributing with a good idea and the nice example :+1:

loopspace commented 3 years ago

Thank you @PhelypeOleinik for that explanation. I've heard of the save stack but never really understood it so that was very useful (and even had I known it then it would still have been a useful contribution!).

I need to think a bit more about how to achieve what I want. I might redesign the structure of the underlying data anyway because, as I understand it, deep down then a prop is a single macro that contains all of its information. In an spath object then one bit of information (the path) might well be quite large and if I'm changing bits of the spath object but not the path then it feels a bit heavy to drag it around. So I'm now wondering about having each part of the spath object being stored in a separate macro and the central prop simply containing the names of those macros (or possibly not even that as I could use a naming scheme such as \l__spath_<name>_<attribute>_<type> so not need a central organising object). This might also mean that I can have local and global versions of each attribute since the getters and setters are my own functions and so the global setter can overwrite the local attribute as well as the global one.

Right, so how about that?

In case that's not clear, here's my proposal:

  1. Each attribute is associated to two macros of the form \x__spath_<name>_<attribute>_<type> where x is each of l and g.
  2. The local setter only sets the l variant but the global one changes both g and l (or it could clear the l one to avoid duplication).
  3. The local getter tries to get the l variant but if that doesn't exist it gets the g one.

Can either of you see what problems I might run into with that?

PhelypeOleinik commented 3 years ago

as I understand it, deep down then a prop is a single macro that contains all of its information

That is correct, yes. At the beginning of this answer I explain the underlying structure of a prop.

In an spath object then one bit of information (the path) might well be quite large and if I'm changing bits of the spath object but not the path then it feels a bit heavy to drag it around

It is probably not a bottleneck, as prop operations are quite optimised, but yes, if there are 100 of items in the prop and you always skip the first 99 to access the last one, you will have a little bit of wasted time there, because TeX has to look at everything before finding what you want. That said, this scanning is done using a delimited macro, so it is as fast as TeX allows.

So I'm now wondering about having each part of the spath object being stored in a separate macro and the central prop simply containing the names of those macros (or possibly not even that as I could use a naming scheme such as \l__spath_<name>_<attribute>_<type> so not need a central organising object).

This feels a bit like what we have in lthooks. We started with a prop for the sorting rules, so that with \DeclareHookRule{myhook}{pkg-a}{before}{pkg-b} you'd have a prop for myhook with an item called pkg-a|pkg-b that contained before, but eventually I replaced that by a macro \g__hook_myhook_rule_pkg-a|pkg-b_tl that expands to before.

The only problem (and not exactly a problem) with this approach is that, in the case of hook rules, I need a list of all the code labels (pkg-a and pkg-b in the example) that exist in myhook, so I know how to access them using a \csname. In your case you will need to know all possible <name>s and <attribute>s or have them listed somewhere so that you can look them up. Other than that, this approach should behave more or less the same as the one with a prop list.

  1. The local setter only sets the l variant but the global one changes both g and l (or it could clear the l one to avoid duplication).

With that setup, your local setter must actually do a global assignment to the l variable, otherwise you'd be back to square one with the global setter doing a global assignment to l (so you'd still need to suspend and resume debugging, or use (say) L for the local variable, so l3debug doesn't complain). The, for that to work, you'd need to manually (globally) restore the l variable at the \endgroups, so that for the user they look like a local setting. This is easy if the \endgroups are from the code in spath... not so much if they are in the user's document (you'd need to use \aftergroup to add your bookkeeping).

With this setup you are basically ignoring TeX's grouping and doing your own macro-level global/local distinction and save stack. Sounds fun ;-)

  1. The local getter tries to get the l variant but if that doesn't exist it gets the g one.

I don't think you need a local and a global getter (if that's what you meant), the same way TeX doesn't have one. You'd need to keep one of l or g cleared (or with a special marker) and use which one was assigned last. Since you will do global assignments to both, this is not hard (the harder part is to restore at the end of a group).

All in all, I think your approach is doable and should work (I see no reason for it to not work), but it's quite a bit of work.

If you want a code review or any other input in this endeavour, feel free to ping me

loopspace commented 3 years ago

I haven't retried it with the debug settings, but I have considerably refactored the code in such a way as to address some of the issues.

It is now a based on a functional paradigm rather than the original object-oriented approach. This means that it works on macros directly and so leaves up to the user the issue of what is local and global.

This is quite a major change, but the only packages that directly rely on the original code are mine (the knot, calligraphy, and penrose libraries) so it shouldn't affect anything that I don't have control over.

The example files spath3_test.tex, knots_test.tex, and calligraphy_test.tex all compile with the new code.

I'm not sure that there's anything particular for either of you to test at this stage but just wanted to update you with where I'm at. I'll give it a go with the debug settings next and report back.

loopspace commented 3 years ago

@PhelypeOleinik and @pablgonz , the latest version passes the debug tests when I run them on my test files.

This feels like a version that, once fully tested, I'd like to get uploaded to CTAN. If either of you would be willing to take a look to see if there's anything obvious I've done that should be changed before I do so, that would be fabulous (but, of course, there's no expectation that you do so!).

Many thanks for all the advice you've given me thus far.

PhelypeOleinik commented 3 years ago

Wow, that was a complete rewrite of the code! I must say I'm (very positively) impressed!

I took a look at the code, and expl3-wise it looks good. I won't say I understand what it does neither how it works, so I can't say anything about that (maybe @pablgonz is more qualified that me here). But if it does what you need (and as you say it is mainly internal for your packages), I think it is fine.

\begin{l3build-advertisement} Do you have a test suite for your packages? \end{l3build-advertisement}

loopspace commented 3 years ago

Thanks, @PhelypeOleinik .

I don't have a test suite, but several times while re-writing then I felt that I ought to. Is there any guidance on what one looks like in L3? Or an example that I could look at to see how to make one?

PhelypeOleinik commented 3 years ago

I suggest you do not start with the ones from latex2e or latex3, as those scripts are for large bundles, and likely too complicated (I know, I tried :).

Maybe an easy start is a setup similar to what I have for namedef. There I have this build.lua script:

#!/usr/bin/env texlua

module = "namedef"

sourcefiles   = { "*.dtx", "*.ins" } -- Files for unpacking and testing

typesetfiles  = { "*.dtx" } -- Files for typesetting docs

checkruns     = 1
typesetruns   = 3

checkengines  = { "pdftex" }

if not release_date then
   dofile(kpse.lookup("l3build.lua"))
end

then a folder testfiles with pairs of .lvt (.tex) and .tlg (.log) files for l3build to run on, and a support folder with regression-test.tex, which is just a copy of the one distributed with l3build.

Running l3build check on the root folder runs each of the .lvt files, and compares the .log file generated by TeX with the reference .tlg file.

If you need any help, you can ping me here or contact be my mail (the address is in the .dtx file in that repository).

pablgonz commented 3 years ago

@loopspace The work looks fabulous :) ... it took me a little while to make the tests, I assume you work with make to make them locally, as a comment followed by @PhelypeOleinik , if you want to move to l3build maybe you can use it (https://github.com/pablgonz/demopkg-jw), I tried but the structure of your repo doesn't fit much to l3buld. Thank you for the tremendous work. Saludos PD: I don't know about any regression test :(

loopspace commented 3 years ago

The test stuff looks interesting, but needs more investigating than I have time for right at the moment (I've started a rudimentary test file but using, I think, a different paradigm on the basis that some testing is better than none).

However, as the original issue has now been resolved then for housekeeping reasons I'm going to close this with, again, many thanks to both of you for your help and insight.

pablgonz commented 3 years ago

Just for fun, I tweaked a bit the example files you have here (I removed shellsc) and moved everything to l3build in my fork :) I noticed that some examples don't compile with the new code, specifically:

"spath_interface.tex",--! Package pgfkeys Error: I do not know the key '/tikz/save spath',
"knots_test.tex", -- hobby :) ! LaTeX3 Error: Inconsistent local/global assignment
"trefoil_moves_new.tex",-- ! Package tikz Error: Sorry, the system call 'pdflatex -shell-escape -halt-on-e
"findintersection.tex",-- Package pgfkeys Error: I do not know the key '/tikz/split at self intersections', to which you passed '{coil}{1111111111111111111111}'
"knotsarrows.tex",--  ! Missing number, treated as zero. \__dim_eval_end:
"renderpath.tex",-- ! Package pgfkeys Error: I do not know the key '/tikz/save spath',
"knot_shading.tex",--! Undefined control sequence.<argument> \g__prg_map_int
"smallcrossings.tex",--! Package tikz Error: Sorry, the system call 'pdflatex -shell-escape -halt-on-e
"knightstour.tex",-- ! Package pgfkeys Error: I do not know the key '/tikz/save spath'
"spath3_test.tex",-- hobby :) ! LaTeX3 Error: Inconsistent local/global assignment
"welding.tex",-- ! Package pgfkeys Error: I do not know the key '/tikz/save spath',

(maybe is a fake psitive because I only used pdflatex).

If you need the l3build model that I have set up (https://github.com/pablgonz/spath3), I can do a PR, I configured it with the elemntal for testing and installations:

Just to say thanks for the great work on this :) Saludos Pablo

loopspace commented 3 years ago

I should probably clean up the examples directory a bit. It's mainly a place where I test some specific instance of a package to see if I can fix a problem (usually that's been raised on TeX-SX). But once the specific problem has been fixed then I often don't go back to that example when further updating.

It's quite useful to me to have these old documents around so that I can see what previous issues have been, so probably the easiest thing to do would be to add a README to that directory to say which files should be viewed as current and which are out of date.

The local/global issue with hobby is something I know about, but there's a bigger re-write planned for that package to make use of new array structures in L3 (I had to invent my own when I originally wrote it!).

loopspace commented 3 years ago

I've added a readme to the examples directory that classifies the various files there, and one or two have been updated to work with the current version. At some point I'll probably create subdirectories to make the distinction more obvious.

I had a look at the hobby package and realised that I've fixed the local/global issue in the development version. I didn't upload that version to CTAN as I was anticipating some changed to the way I did arrays - Joseph Wright once said he'd take a look and see if he could advise me on that. Looking at it now, none of the low-level objects in L3 have all the structure that I need so I'm not sure it's worth changing it now just to have to change it again later when someone implements true arrays in L3. So I think I'll do some test runs on that and upload the current version to CTAN. I'll open an issue there to keep track of anything relevant to that package.

PhelypeOleinik commented 3 years ago

@loopspace What type of array do you need? I mean in the sense of what do you need from an array type, if it were to be provided by the kernel? (As you probably know, writing the actual code is not the hard part, but coming up with sensible specification and interfaces, so use cases are most welcome :)

loopspace commented 3 years ago

As the discussion of arrays is more pertinent to hobby, I've opened an issue there with details. It's at https://github.com/loopspace/hobby/issues/8 . I would welcome any advice and guidance from you both.