-
-
Notifications
You must be signed in to change notification settings - Fork 487
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Colormaps and contours for complex_plot #33416
Comments
Author: David Lowry-Duda |
Changed keywords from none to complex plotting |
This comment has been minimized.
This comment has been minimized.
Commit: |
comment:4
Thank you for the contribution. Some comments:
|
comment:5
Thank you for this addition. This will be nice to have in Sage. I have been playing around with this and have some remarks as well.
diff --git a/src/sage/plot/complex_plot.pyx b/src/sage/plot/complex_plot.pyx
index a3551584ce..d116887b83 100644
--- a/src/sage/plot/complex_plot.pyx
+++ b/src/sage/plot/complex_plot.pyx
@@ -387,14 +387,13 @@ def cmap_complex_to_rgb(z_values, cmap=None, contoured=False, tiled=False):
normalized_colors = cmap((args + PI) / (2 * PI))
normalized_colors = normalized_colors[:,:,:3] # discard alpha channel
lightdeltas = als[:,:,1]
- lightdeltas = lightdeltas # normalize lightness adjustment
- arg_d_s = np.dstack((normalized_colors, lightdeltas))
if tiled or contoured:
+ arg_d_s = np.dstack((normalized_colors, lightdeltas))
# add contours: convert to hls, adjust lightness, convert back
rgbs = manual_contoured_cmap_complex_to_rgb(arg_d_s)
else:
- rgbs = manual_smooth_cmap_complex_to_rgb(arg_d_s)
+ rgbs = manual_smooth_cmap_complex_to_rgb(normalized_colors, lightdeltas)
# Apply mask, making nan_indices white
rgbs[nan_indices] = 1
@@ -451,7 +450,7 @@ cdef inline double clamp(double x):
return x
-def manual_smooth_cmap_complex_to_rgb(rgb_d_s):
+def manual_smooth_cmap_complex_to_rgb(rgb, delta):
"""
Returns an rgb array from given array of `(r, g, b, delta)`.
@@ -508,30 +507,10 @@ def manual_smooth_cmap_complex_to_rgb(rgb_d_s):
array([[[0.75 , 0.8125, 0.875 ]]])
"""
import numpy as np
-
- cdef unsigned int i, j, imax, jmax
- cdef double r, g, b, delta, h, l, s
-
- imax = len(rgb_d_s)
- jmax = len(rgb_d_s[0])
-
- cdef cnumpy.ndarray[cnumpy.float_t, ndim=3, mode='c'] rgb = np.empty(dtype=float, shape=(imax, jmax, 3))
-
- sig_on()
- for i from 0 <= i < imax:
- row = rgb_d_s[i]
- for j from 0 <= j < jmax:
- r, g, b, delta = row[j]
- delta = 0.5 + delta/2.0
- if delta < 0.5:
- rgb[i][j][0] = clamp(2 * delta * r)
- rgb[i][j][1] = clamp(2 * delta * g)
- rgb[i][j][2] = clamp(2 * delta * b)
- else:
- rgb[i][j][0] = clamp(2*(delta - 1.0)*(1 - r) + 1)
- rgb[i][j][1] = clamp(2*(delta - 1.0)*(1 - g) + 1)
- rgb[i][j][2] = clamp(2*(delta - 1.0)*(1 - b) + 1)
- sig_off()
+ delta = delta[:,:,np.newaxis]
+ delta_pos = delta > 0.0
+ rgb = (1.0 - np.abs(delta))*(rgb - delta_pos) + delta_pos
+ rgb = np.clip(rgb, 0.0, 1.0)
return rgb
Now, this is not much slower than the old implementation anymore:
previously:
At this point, I think it makes sense to completely remove the old implementation and replace it by using the colormap
The
|
Branch pushed to git repo; I updated commit sha1. New commits:
|
comment:7
Replying to @tscrim: Thank you for your suggestions and the documentation fixes. I've incorporated these changes.
It's not possible (as far as I can tell) to give the user the ability to choose the parameter for scaling. For example, I chose contours at The reason it's nontrivial to include is that these are used heavily in the compiled portion of the cython. I think this is the same reason why there is no runtime option to change the exponent for Of course, I could be wrong.
Oh, thank you! I hadn't realized that cython changed to convert range-based loops into C-loops. (It seems this happened in 2018, and I'm behind the times). This is much nicer! I'm currently examining the comments from @mwageringel, but haven't acted on those comments yet. New commits:
|
comment:8
Replying to @davidlowryduda:
Someone might utilize that difference. With having an extra parameter that the user can control, this becomes a useful tool that I believe should be easy to implement.
This doesn't make sense to me. You can just make your hardcoded values into input parameters, or have the lightness be a function (with the default being the hardcoded function). |
comment:9
Replying to @tscrim:
I don't know what I was thinking. Of course you're right. This is easy to implement. I'll do that too. |
Example of aliasing artifacts for linear contours |
comment:11
Attachment: result.png I have now included several of the suggestions from @mwageringel. In particular, I have changed many of the implementations to now be in numpy, including writing numpy forms of HLS->RGB and RGB->HLS translations. I'm not particularly great at numpy, but nonetheless this is markedly faster than my previous version.
I almost did this completely. I changed the default
This seems close enough to me that I'd be happy to remove the old implementation entirely.
These commits also include this implementation. It is now possible to choose between linear vs log scaling, to choose the intervals between contours, the number of phases in the tiling method, and the rate of change of magnitude when there are no contours. I note that linear scaling can often lead quickly to Moire patterns once the rate of change of the function gets to be large. For a concrete example, I've included the two outputs of the plots from
I'll add one such example and a warning to the documentation. It is also now possible to adjust the exponent controlling how quickly the magnitude tends to white or black. This can be seen for example in
|
Attachment: twilight.png twilight example |
comment:12
Replying to @mwageringel:
The output looks about as I expect. For concreteness, I've included a version (with 300 plot points) here: There are two interactions governing the colors. First, there are contours affecting magnitudes. Then there is the colormap. The phase is mapped to the color, so we should expect on any circle centered at the origin to go through a cyclic continuous colormap (but each circle might be adjusted in lightness by the contouring). In a commit that will immediately after I write this, it's also possible to choose contour visibility (by adjusting
I think this is a good idea! I think I'm going to add many examples. |
Branch pushed to git repo; I updated commit sha1. New commits:
|
comment:14
Great, thank you. Looks very pretty. I just have a few trivial doc comments: And similar: .. SEEALSO::
- :func:`sage.plot.complex_plot.mag_to_lightness`
- :func:`sage.plot.complex_plot.mag_and_arg_to_lightness`
- :func:`sage.plot.complex_plot.cyclic_linear_mag_to_lightness`
+ - :func:`sage.plot.complex_plot.mag_to_lightness`
+ - :func:`sage.plot.complex_plot.mag_and_arg_to_lightness`
+ - :func:`sage.plot.complex_plot.cyclic_linear_mag_to_lightness` For
Instead of using |
comment:15
Replying to @davidlowryduda:
The difference is very small, but I cannot fully explain where the difference comes from. On closer inspection, I also noticed that the matplotlib
A way around this would be creating our own hsv color map with more than 256 sampling points, but maybe that goes too far for this ticket. For now, should we just keep the old implementation? Replying to @davidlowryduda:
Yes, I now obtain the same result. When I reported the problem, there was a discontinuity in the color spectrum at about 10 degrees, but it seems to be fixed now. I still need to take a closer look at the implementation. |
comment:16
I wouldn’t worry too much about minor aliasing differences when there is slight variation (some monitors might not be able to resolve these differences either). That being said, I am not opposed to keeping the current implementation since it does provide slightly better performance and it is already there and working. |
Branch pushed to git repo; I updated commit sha1. New commits:
|
Changed branch from u/DavidLowry/colormaps_and_contours_for_complex_plot to u/gh-mwageringel/33416 |
comment:27
Thank you for all the changes. I have fixed one more typo in the documentation and have also run the tests locally. New commits:
|
comment:28
|
comment:29
Thank you, Volker. @davidlowry: To explain the problem a bit further, the problem with this test is that it is not numerically stable. I cannot replicate the problem directly, but adding a small perturbation to one of the values demonstrates it:
Probably, Volker observed the issue on a 32-bit machine, which can lead to numerical differences like this. Is this a problem with the test or with the implementation, i.e. should the implementation of |
comment:30
Oh, thank you Markus. I couldn't replicate this and didn't understand. But you're right! Looking at this test case now, I see that this is a poor choice of parameter. I think I chose them to be "random small numbers", but the law of small numbers bites here. The reason this test acting like this is because a rgb value is being associated to a point lying exactly on a contour boundary. Here, contours are associated on powers of Away from the contours, these tests should be fine and pretty stable. I'll replace this test with a different one. I'll also make sure that I didn't make this same error elsewhere. |
Changed branch from u/gh-mwageringel/33416 to u/DavidLowry/33416 |
New commits:
|
comment:33
I actually wonder how much we have to worry about numerical noise. Perhaps we can modify these doctests to not have explicit values? Maybe just the first two or three sigfigs with |
Branch pushed to git repo; I updated commit sha1. New commits:
|
comment:35
@tscrim That makes sense to me. I've incorporated a pretty general absolute tolerance into the tests. |
comment:36
Thanks. I changed some numbers to verify that the tolerance works. Two last little changes: The doctests on lines 1104 and 1114 should be marked as |
comment:37
Minor suggestion: .. [LD2021] David Lowry-Duda.
*Visualizing modular forms*.
- Arithmetic Geometry, Number Theory, and Computation, Simons
- Symposia, Springer, 2021.
+ Arithmetic Geometry, Number Theory, and Computation.
+ Simons Symposia, Springer, 2021.
:arxiv:`2002.05234` |
Branch pushed to git repo; I updated commit sha1. New commits:
|
Changed branch from u/DavidLowry/33416 to |
This ticket rewrites
complex_plot
to accept an optional matplotlib-compatible colormap and optional arguments that change the domain coloring to include "contours". I've been using this code in https://github.com/davidlowryduda/phase_mag_plot, and a (slower, older) variant of this code were used in my paper https://arxiv.org/abs/2002.05234.Typical usage would go like
To include magnitude-based contours, one would write something like
This also includes the domain coloring tiling patterns from
Wegert and Semmler, Phase plots of complex func-
tions: a journey in illustration (Notices AMS 2010). This would be written like
Colormaps and contours can be combined or used separately. Without these arguments, the previous
complex_plot
functionality continues unchanged.Component: graphics
Keywords: complex plotting
Author: David Lowry-Duda
Branch/Commit:
e1b7ca9
Reviewer: Markus Wageringel, Travis Scrimshaw
Issue created by migration from https://trac.sagemath.org/ticket/33416
The text was updated successfully, but these errors were encountered: