feat: add colours to ModFill

[katestange]

By submitting this PR, I am indicating to the Numberscope maintainers that I have read and understood the contributing guidelines and that this PR follows those guidelines to the best of my knowledge. I have also read the pull request checklist and followed the instructions therein.

---

This is still marked draft but feedback is welcome. It adds several colouring abilities to ModFill:

1. options to choose colour and alpha (transparency) for drawing, transparency possibly as function of modulus
2. option to highlight certain terms by using a second colour; a formula box allows you to pick which terms to highlight by a formula on the index, e.g. isPrime(n), and relevant controls
3. removes boring ModFill gallery items and adds nice ones (in flux again now)
4. updates documentation (still needs images)
5. adds background colour option as well as aspect ratio option

Things to do:

• update snapshots
• update images in docs
• there's a background colour option that should work once the sequence cache bug is fixed
• update gallery
• consider other colour options
• consider making some parameters appear when highlighting formula is not default

[katestange]

Ok sorry for the rebase mess, this one should be clean but I see now something is wrong -- presumably something to do with my new formula parameter and your formula changes. I'll take a look at that when I get a chance.

[gwhitney]

OK, just let me know when you're ready for me to look.

[katestange]

Ok, the merge issue is fixed, it should run and be ready except for the snapshots.

[gwhitney]

First some general high-level feedback:

1. Given that you are diving into color schemes, have you reviewed Add color schemes to Mod Fill visualizer #143? This PR more or less implements scheme 3 there, and then it has the highlight color not mentioned there. Are any of the other color schemes worth implementing now? This scheme allows one to use a saturation gradient for the number of times that a residue has occurred; would you want to enable the visualizer to optionally use a hue gradient instead?
2. I'm thinking you are trying to keep this focused, and not considering it the complete overhaul, so you would want to leave consideration of possibly doing Mod Fill should have legend #309 and/or ModFill should have mouseover #386 to another PR, correct?
3. In a similar vein, you want to leave the question of whether to always have ModFill be square, or to have a toggle that lets you flip between "square" and "fill the rectangle", to another PR? (I definitely think that square should at least be an option.)
4. I've never loved the description "A triangular grid". To me that calls to mind the tessellation of equilateral triangles. Can you think of better/clearer wording? Possibly just remove the word "triangular"?
5. Should the default for "Highlight Formula" be false, corresponding to the good ol' monochrome behavior? If we changed it to default to 0, the description could suggest isPrime(n) as an example.
6. When "Highlight Formula" is false or 0, should the Highlight color be hidden, since it won't be used?
7. On Featured specimens, does ModFill warrant three? Is there any total number we're more or less going for, or average per visualizer? If you wanted to prune one, the 'Prime Residues' one seems easiest to discover on one's own, so maybe has the least need to be in featured specimens. (And I know I am undermining my own point here but I sort of miss Residue Rise -- it made a really interesting 3D-looking texture after a while, which i am not sure the reason for. So maybe I am actually barking up the wrong tree, and we should be tending toward a whole lot of featured specimens, and I should just be campaigning for Residue Rise to be left in... What's your reaction on this point?)
8. Not sure why transparency is an integer from 1 to 255; wouldn't it be more typical to make it a decimal between 0 and 1? In particular, in the Chaos visualizer, we call this same feature "Alpha" and have it be between 0 and 1. I think the "Transparency" label is clearer, and the 0 - 1 dynamic range is clearer, but in any case, I think most strongly of all that the two should be completely consistent with each other.

[gwhitney]

One more general philosophical question here: When the transparency level is low, the bottom-left corner is always darker than the right-hand side, simply because there are fewer residues so each on average occursmore times. In many cases, this is visual noise, there's not really "more hits" in the smaller residues than the larger residues, in that each term does in fact have one residue per modulus; they're just spread out over more different residues in the higher moduli. Should there be an option, perhaps even an option turned on by default, to allow the transparency to depend on n so that you can adjust things so that "all else being equal", the diagram would be uniformly saturated left to right?

[gwhitney]

More small-bore comments (as with most of my feedback, more questions than directives):

1. Would 'Highest modulus to display' be clearer as the label instead of just 'Highest modulus'?
2. I think the term "hit" is unclear in "Transparency of each hit". Is there better wording? Is "Transparency of each residue plotted" any better? "Transparency of each occurrence of a residue"?
3. The capitalization in "Highlight Formula" is inconsistent with other fields; "Highlight formula" would be more consistent. But would a label like "Indices to highlight" be clearer/more informative? You can explain it is a formula in the description (which you basically do already).
4. There's a space missing after 'of' in 'residue ofa(n)' in the description for this parameter.
5. Logically, the highlight transparency could be different from the usual one. Do you want to add a parameter for that? If there were an RGBA color picker, it would be nice to just use that for both...

[gwhitney]

I searched around. There is no browser-native rgba color picker, but it appears that it would be pretty easy to create a RGBA param type and roll our own input setup with a combination of a color picker and 0-1 slider. If you think that would be useful, I am happy to help. We could use the rgba color in Turtle, too, I suppose -- Wait For It would definitely look different with some transparency for the turtle path, for example.

[gwhitney]

Code is all nice and straightforward. Once the design/wording stuff above is decided/resolved, we should be good to go.

[katestange]

I searched around. There is no browser-native rgba color picker, but it appears that it would be pretty easy to create a RGBA param type and roll our own input setup with a combination of a color picker and 0-1 slider. If you think that would be useful, I am happy to help. We could use the rgba color in Turtle, too, I suppose -- Wait For It would definitely look different with some transparency for the turtle path, for example.

I think this would be lovely! Yes, Turtle would benefit too, and it would simplify parameter panels (although it hides powerful functionality in some cases, so we might want to add a hint or something). Would the roll our own basically use the browser RGB plus a slider? Do you imagine doing that as part of this PR or another one?

[katestange]

I need help with a weird (race condition?) bug. Try this URL:
http://localhost:5173/?name=draw+problems&viz=ModFill&modDimension=100&alpha=100&fillColor=a51d2d&highlightFormula=0&highColor=ff7800&seq=Formula&formula=floor%28n%2F1%29
You'll see (pic attached from firefox), that the bottom row(s) isn't/aren't always drawn at first. What's happening, as far as I can tell, is that code is running and thinks it is running (console logs confirm this), but drawing is not happening on canvas, until some sort of delay time has passed. For example, if you put a command to change background colour, it won't happen unless you program it to happen after some number of terms. It seems (??) worse with floor(n/1) than with n (which should be equivalent) and worse (??) on firefox than chrome, but it is intermittent how many rectangles are missed. I'm stumped.

[gwhitney]

I need help with a weird (race condition?) bug.

For me it is only happening on page load. If I click your URL and then enter a "2" in the number of terms to use, I get both horizontal stripes. Similarly if I change any other parameter of the the visualizer i seem to be getting all of the horizontal stripes. Does that mesh with what is happening for you?

[gwhitney]

Wow, and I agree, I can't get it to happen with formula n but it reliably does on page load with formula n/1. And it only seems to be happening on ModFill, in that with formula n/1 for two terms I get two strokes in Turtle, two dots in Chaos, etc. Does that also match with your experience? So weird...

[gwhitney]

Finally some progress, but now a different mystery. Somehow the cache for formula n/1 is empty in the first call to draw(), so it's throwing a caching error, which is why nothing is being drawn. The remaining mystery is that for n, the cache is fine (first 128 values filled by the time you get to draw()). Why would there be a difference?

[gwhitney]

OK, it's an async initialization mess with sequences. Roughly the problem is that Specimen.makeSequence returns right away, but it has initiated an asynchronous validate, which may eventually conclude that the parameters have changed, and that it needs to initialize the sequence, but that might end up happening after the sequence has filled its cache, and then the cache gets destroyed... Will try to untangle, but the sequence of events is really muddled. This is not a proud part of Numberscope's design :-(

[katestange]

OK, it's an async initialization mess with sequences. Roughly the problem is that Specimen.makeSequence returns right away, but it has initiated an asynchronous validate, which may eventually conclude that the parameters have changed, and that it needs to initialize the sequence, but that might end up happening after the sequence has filled its cache, and then the cache gets destroyed... Will try to untangle, but the sequence of events is really muddled. This is not a proud part of Numberscope's design :-(

Oh wow, thank you for figuring that out! I thought maybe it was something beyond this visualizer. But why doesn't it happen to Turtle etc?

[gwhitney]

Not sure why it isn't happening to other visualizers but I think it is because of a quasi-bug in modfill that we should fix: it increments its index before draw has succeeded. Since eventually there will be a cache miss, that's not actually kosher. But it has been a useful quasibug, in that there shouldn't be a cache miss on the first call to compute

[katestange]

1. Given that you are diving into color schemes, have you reviewed [Add color schemes to Mod Fill visualizer #143](https://github.com/numberscope/frontscope/issues/143)? This PR more or less implements scheme 3 there, and then it has the highlight color not mentioned there. Are any of the other color schemes worth implementing now? This scheme allows one to use a saturation gradient for the number of times that a residue has occurred; would you want to enable the visualizer to optionally use a hue gradient instead?

So far haven't tackled more of this.

2. I'm thinking you are trying to keep this focused, and not considering it the complete overhaul, so you would want to leave consideration of possibly doing [Mod Fill should have legend #309](https://github.com/numberscope/frontscope/issues/309) and/or [ModFill should have mouseover #386](https://github.com/numberscope/frontscope/issues/386) to another PR, correct?

Correct.

3. In a similar vein, you want to leave the question of whether to always have ModFill be square, or to have a toggle that lets you flip between "square" and "fill the rectangle", to another PR? (I definitely think that square should at least be an option.)

Done.

4. I've never loved the description "A triangular grid". To me that calls to mind the tessellation of equilateral triangles. Can you think of better/clearer wording? Possibly just remove the word "triangular"?

Tried to improve.

5. Should the default for "Highlight Formula" be `false`, corresponding to the good ol' monochrome behavior? If we changed it to default to 0, the description could suggest `isPrime(n)` as an example.

Done.

6. When "Highlight Formula" is `false` or `0`, should the Highlight color be hidden, since it won't be used?

Not sure how to do this -- currently the params system doesn't have something like that. Should I consider adding it? Seems useful.

7. On Featured specimens, does ModFill warrant three? Is there any total number we're more or less going for, or average per visualizer? If you wanted to prune one, the 'Prime Residues' one seems easiest to discover on one's own, so maybe has the least need to be in featured specimens.  (And I know I am undermining my own point here but I sort of miss Residue Rise -- it made a really interesting 3D-looking texture after a while, which i am not sure the reason for. So maybe I am actually barking up the wrong tree, and we should be tending toward a whole lot of featured specimens, and I should just be campaigning for Residue Rise to be left in... What's your reaction on this point?)

Not addressed right now (and this most recent commit doesn't change the gallery which is a bit broken by this commit.

8. Not sure why transparency is an integer from 1 to 255; wouldn't it be more typical to make it a decimal between 0 and 1?  In particular, in the Chaos visualizer, we call this same feature "Alpha" and have it be between 0 and 1. I think the "Transparency" label is clearer, and the 0 - 1 dynamic range is clearer, but in any case, I think most strongly of all that the two should be completely consistent with each other.

Done.

[katestange]

One more general philosophical question here: When the transparency level is low, the bottom-left corner is always darker than the right-hand side, simply because there are fewer residues so each on average occursmore times. In many cases, this is visual noise, there's not really "more hits" in the smaller residues than the larger residues, in that each term does in fact have one residue per modulus; they're just spread out over more different residues in the higher moduli. Should there be an option, perhaps even an option turned on by default, to allow the transparency to depend on n so that you can adjust things so that "all else being equal", the diagram would be uniformly saturated left to right?

I made transparency a value that could also be a function of m. That way, someone can play with this. But in my experiments, the way it is set by default is the most visually pleasing. You can try a few combos, and if there are good ones we can add them to the docs.

[katestange]

1. Would 'Highest modulus to display' be clearer as the label instead of just 'Highest modulus'?

Tried a reword.

2. I think the term "hit" is unclear in "Transparency of each hit". Is there better wording? Is "Transparency of each residue plotted" any better? "Transparency of each occurrence of a residue"?

Tried to improve.

3. The capitalization in "Highlight Formula" is inconsistent with other fields; "Highlight formula" would be more consistent. But would a label like "Indices to highlight" be clearer/more informative? You can explain it is a formula in the description (which you basically do already).

Tried a new wording.

4. There's a space missing after 'of' in 'residue ofa(n)' in the description for this parameter.

Fixed.

5. Logically, the highlight transparency could be different from the usual one. Do you want to add a parameter for that? If there were an RGBA color picker, it would be nice to just use that for both...

This is a cool idea, but now that I made transparency possibly a formula in m, modulus, this would be undoing that capability.

[katestange]

I have updated the PR checklist to show the items that are currently outstanding.

[katestange]

Not sure why it isn't happening to other visualizers but I think it is because of a quasi-bug in modfill that we should fix: it increments its index before draw has succeeded. Since eventually there will be a cache miss, that's not actually kosher. But it has been a useful quasibug, in that there shouldn't be a cache miss on the first call to compute

I believe I fixed this quasibug, but the underlying cache bug still remains.

[katestange]

Here's an example where a formula is still popping up some errors while trying to type: backspacing the variable inside "isPrime(a)" produces a pop-up, but only for the formula field in ModFill, not for the formula field in Sequence. Do you know how to easily fix this (seeing as you were just fixing this in another PR)?
http://localhost:5173/?name=Formula%3A+n&viz=ModFill&modDimension=13&backgroundColor=000000&fillColor=1a5fb4&alpha=0.01&aspectRatio=1&highlightFormula=isPrime%28a%29&highColor=f66151&alphaHigh=0.4&seq=Formula

[katestange]

My newest commit allows for variables n (index) and a (value) in the highlight formula. I also added checking whether they were actually in the parsetree and avoiding even trying math.safeNumber() on things that aren't in the parsetree, so as to avoid pointless failures. This is necessary, for example, if you want to use highlight formula involving n but not a on VF numbers, where a is not a safe number after just 79 terms, but n is fine.

[gwhitney]

OK, I'm going to take a quick look at this bug insofar as I can given that it's not happening on my machine. When you get a chance, let me know what exact version of Firefox you are running (and/or try updating to as recent a version as you can, but check the version number before you update). Also please check: when you have the specimen as the screenshots above, with the number of terms set to 2 (i.e., highest number where you see nothing), if you quickly resize the browser, do you see any flashes of black, or of blue stripes near the bottom of the canvas, i.e. glimpses that it is drawing only to get wiped out by new white. In the meantime, I will try to follow up on the one weirdness I can observe here, which is that it seems to be initializing twice on page reload, when it seems like once would suffice. I speculate that maybe if I could get rid of the double initializing here, then it might start displaying properly on your end.

[gwhitney]

Oh wait, I take it back. With the latest code, I only seem to be initializing once. So please do post that further information when you get a chance, and then if that doesn't ring any bells, I don't see any option but a joint debugging session when we can find the time, since nothing tangible awry is happening here :-/

[katestange]

The bug has apparently disappeared (?!??!!?) so we forge onwards!

[gwhitney]

OK, passing the baton back. Let me know how that works for you, in terms of error checking in all formula entry boxes and using the Opacity formula if there is no Highlight Opacity formula.

[gwhitney]

Incidentally, min(0.4, 0.01*m) might be an even better saturation equalizer than k*log(m), see what you think...

[katestange]

@gwhitney I don't know why, but the "changing a sequence" e2e test is timing out. Are you seeing this?

[gwhitney]

Can't run tests right now since internet on plane is too slow. But there is one test -- I can't remember which -- that erratically times out for me (roughly one time in 3 or 4). It always passes on a re-run, in my experience. If you are getting one consistently timing out, try running just that one and then looking at the web report to see if you can see what's going on. If you're still stuck next time I have some time to look at this (likely Friday), I will poke around.

[katestange]

Interestingly your bug fix resulted in a 2 pixel difference in the Chaos snapshots. So there was something (undetectable) wrong happening from it. :)

[katestange]

Ok, I think this is ready for review. I think it's a big enough PR, that I've not implemented any new colour schemes or features in this latest round. If you'd like to keep something for future, let me know and we can file an issue or you could feel free to add something or request something. But nothing seems like a burning need for me right now, so I'm marking it ready.

[katestange]

Also, I must add, it never ceases to amaze me how a PR I think will be small -- some colour changes in ModFill -- will end up touching 60 files! I guess I see why you try not to overlap PRs and rebase too much.

[gwhitney]

Also, I must add, it never ceases to amaze me how a PR I think will be small -- some colour changes in ModFill -- will end up touching 60 files! I guess I see why you try not to overlap PRs and rebase too much.

Well, "only" a dozen source files, the rest are all tests. And it's happening because we are still in process of flushing out fundamental bugs, and not being very disciplined about pausing the PR, doing a separate bug fix PR, and then rebasing the feature PR, since we are a small pre-alpha project.

I take it the test timeout stopped bothering you?

[gwhitney]

Interestingly your bug fix resulted in a 2 pixel difference in the Chaos snapshots. So there was something (undetectable) wrong happening from it. :)

Just in the CI snapshots, right?

[katestange]

Just in the CI snapshots, right?

Correct!

[gwhitney]

Ok, I think this is ready for review. I think it's a big enough PR, that I've not implemented any new colour schemes or features in this latest round. If you'd like to keep something for future, let me know and we can file an issue or you could feel free to add something or request something. But nothing seems like a burning need for me right now, so I'm marking it ready.

Right, that seems like a perfectly fine decision. Do any of the other colour scheme suggestions seem like they are worth leaving as issues (meaning we intend to do them at some point)? Or should one of us move them to Discussions (meaning we will think about them someday but have not made any specific decision to necessarily ever implement them)? I am fine either way, just let me know your inclination.

I will review what's here so far when I have the chance, my guess is Fri or Sat.

[gwhitney]

Suggestion: I find the current incarnation of "Picasso's Periods" very strobe-y, to the point of being a bit unpleasant, and the blue/rose "periods" to go by bit too quick. I'd suggest dialing the frame rate down to 30? What do you think? I understand you may not be able to respond for a while; I'll just hold on the review until you do have a chance to get back on this. If you decide you like the decision, I can push the change to defineFeatured if you want (super tiny change); if you decide you'd like to leave it as it is, I will just go ahead with the rest of the review (I already fixed one little bug, where a parameter was not disappearing if the default value of the parameter its visibility depended on was a value on which it should not appear).

[katestange]

Suggestion: I find the current incarnation of "Picasso's Periods" very strobe-y, to the point of being a bit unpleasant, and the blue/rose "periods" to go by bit too quick. I'd suggest dialing the frame rate down to 30? What do you think? I understand you may not be able to respond for a while; I'll just hold on the review until you do have a chance to get back on this. If you decide you like the decision, I can push the change to defineFeatured if you want (super tiny change); if you decide you'd like to leave it as it is, I will just go ahead with the rest of the review (I already fixed one little bug, where a parameter was not disappearing if the default value of the parameter its visibility depended on was a value on which it should not appear).

I think that's a good call. Could even go to 20 if you prefer. Also, you might decrease the opacity on the highlight from 0.4 to 0.3 -- if you think that looks good, you could fiddle with that too. I'd be happy with any of these variations. If you don't mind pushing your favourite variation in this general vicinity, that's great!

[gwhitney]

Wow, I wasn't expecting a response any time soon. I ended up at 24fps, a common speed between 20 and 30; and I didn't mind the "prime bursts" being so bright, but I did ramp up the saturation of the blue periods slightly. Proceeding with review.

[gwhitney]

I have just gone through the PR checklist and all seems good; merging.