Skip to content

Custom colors trisurf #484

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

Merged
merged 2 commits into from
May 27, 2016
Merged

Custom colors trisurf #484

merged 2 commits into from
May 27, 2016

Conversation

choldgraf
Copy link
Contributor

This is a quick add-on to #481 and #482. It basically does 3 things:

  1. Add support for passing a list or array to color_func. In this case, it assumes the list/array has pre-computed color values and it won't try to calculate it from the z-coordinates etc.
  2. Ensure that everything piped into the Mesh3d class is an array. This makes plotting much faster.
  3. Rename the _map_z2color function because we're using this with more than just the z-values (shouldn't affect the functionality at all).

I also added a few tests, which are passing for me. @Kully @theengineear @jackparmer

# Pre-computed list / array of values to map onto color
if len(color_func) != len(simplices):
raise ValueError('If color_func is a list/array, must'
' be the same length as simplices')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐐 (grammar goat), sorry we rely heavily on emojis for code reviews at Plotly.

'If color_func is a list/array, it must be the same length as simplices.'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

haha, will do. I think I left off the it because I was trying to keep the line under PEP8 max, but ended up splitting it anyway. I had a PR last week where someone got me for including a dangling participle :D

@theengineear
Copy link
Contributor

Just a couple of comments @choldgraf, thanks for adding this! 👍 from me after you address those little nit-picks.

@Kully can you also give it a review and pass off if you're happy with it?

@choldgraf
Copy link
Contributor Author

ok, I'll wait until another pair of eyes sees it and will then make changes. Feel free to suggest how to deal with the string checking situation.

For me, the array-forcing got the JSON packaging down to ~4 seconds per hemisphere (down from about 16 seconds).

btw, it looks like the plotly.js repo has been updated with lighting fixes today. Any idea when that'll make its way into the python plotly library?

@jackparmer
Copy link
Contributor

btw, it looks like the plotly.js repo has been updated with lighting fixes today. Any idea when that'll make its way into the python plotly library?

Offline mode should download the latest version of plotly.js from the plotly.js CDN (am I remembering this right @etpinard ?)

@etpinard
Copy link
Contributor

Hmm @jackparmer I don't remember.

@choldgraf
Copy link
Contributor Author

I don't think so anymore. If you look at L34 of offline.py, it says that
downloading is deprecated in favor of shipping a mini plotly.js with
plotly.py

On Thu, May 26, 2016 at 4:50 PM, Étienne Tétreault-Pinard <
[email protected]> wrote:

Hmm @jackparmer https://github.com/jackparmer I don't remember.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#484 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/ABwSHU0UifXxN22fSeNGv6919LD3BmT3ks5qFjHIgaJpZM4Invo8
.

@cldougl
Copy link
Member

cldougl commented May 27, 2016

We're manually updating plotly.js for offline mode.
Here's some reasoning: #348 (comment)

@choldgraf we'll add a new version today with the updates

@choldgraf
Copy link
Contributor Author

thanks! :)

On Fri, May 27, 2016 at 5:31 AM, Chelsea [email protected] wrote:

We're manually updating plotly.js for offline mode.
Here's some reasoning: #348 (comment)
#348 (comment)

@choldgraf https://github.com/choldgraf we'll add a new version today
with the updates


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#484 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/ABwSHeNHPPQ7a6fDm9ATChMW26QsazO2ks5qFuQvgaJpZM4Invo8
.

@Kully
Copy link
Contributor

Kully commented May 27, 2016

New PR with these changes integrated: #486

@Kully Kully merged commit bed542d into plotly:master May 27, 2016
@choldgraf
Copy link
Contributor Author

OK, just tried the latest master and the plotting seems to work as planned. Now just waiting for the JS update!

@Kully
Copy link
Contributor

Kully commented May 27, 2016

OK, just tried the latest master and the plotting seems to work as planned. Now just waiting for the JS update!

There's one thing I noticed that is lacking, and that is support for hex and tuple colors in color_func (if a list).

Right now I am:

  1. adding another example in the trisurf doc for color_func as a list
  2. adding hex and tuple compat. in color_func

I finished 1. but I can't seem to solve tuple compatibility at the moment.

@Kully
Copy link
Contributor

Kully commented May 27, 2016

@choldgraf I am going to make a new PR for what I am doing. But I will hold off on allowing for tuple compatibility. Need to prioritize updating the docs.

@choldgraf
Copy link
Contributor Author

I think it's worth having a handle_colors function that basically allows you to intelligently deal with all of these usecases (rgb strings, rgba strings, hex strings, tuples, an array of rgb 0-1 floats, an array of 0-255 floats, or a 1-D array w/ a colormap specified elsewhere). That'd save lines within the trisurf function and might be useful elsewhere.

@Kully
Copy link
Contributor

Kully commented May 27, 2016

I think it's worth having a handle_colors function that basically allows you to intelligently deal with all of these usecases (rgb strings, rgba strings, hex strings, tuples, an array of rgb 0-1 floats, an array of 0-255 floats, or a 1-D array w/ a colormap specified elsewhere). That'd save lines within the trisurf function and might be useful elsewhere.

That sounds like a good idea. The only thing is that not all functions in FigureFactory use 'rgb' as the canonical color type, to my knowledge. One of them uses array of 0-255 iirc

@choldgraf
Copy link
Contributor Author

Good point - do you want to handle a PR for a function like this? I am
happy to give thoughts, review, etc from my use cases.

On Fri, May 27, 2016 at 1:13 PM, Adam Kulidjian [email protected]
wrote:

I think it's worth having a handle_colors function that basically allows
you to intelligently deal with all of these usecases (rgb strings, rgba
strings, hex strings, tuples, an array of rgb 0-1 floats, an array of 0-255
floats, or a 1-D array w/ a colormap specified elsewhere). That'd save
lines within the trisurf function and might be useful elsewhere.

That sounds like a good idea. The only thing is that not all functions in
FigureFactory use 'rgb' as the canonical color type, to my knowledge. One
of them uses array of 0-255 iirc


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#484 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/ABwSHeJuB-_ZbE4GGImlIZtVvyjl3E-Bks5qF1BYgaJpZM4Invo8
.

@choldgraf
Copy link
Contributor Author

guys this looks awesome

https://www.dropbox.com/s/9ozy6rv92vjr8cg/moarbrainz.mov?dl=0

seriously

@choldgraf
Copy link
Contributor Author

@Kully
Copy link
Contributor

Kully commented May 27, 2016

guys this looks awesome

https://www.dropbox.com/s/9ozy6rv92vjr8cg/moarbrainz.mov?dl=0

seriously

Dayummmm!

@monfera
Copy link

monfera commented May 28, 2016

@choldgraf we made an example for darkened deep grooves: intensity or color gradient depends on the euclidean distance from the mean of the mesh face center coordinates - the squared distances in the pythagorean root are weighted in proportion of the overall brain width per dimension. The code is quick and dirty, just serving as a rough PoC.

    function faceCentroid(x,y,z,i,j,k) {
        var X = 0;
        var Y = 0;
        var Z = 0;
        var faceCount = i.length;
        var f;
        for(f = 0; f < faceCount; f++) {
            X += (x[i[f]] + x[j[f]] + x[k[f]]) / 3;
            Y += (y[i[f]] + y[j[f]] + y[k[f]]) / 3;
            Z += (z[i[f]] + z[j[f]] + z[k[f]]) / 3;
        }
        return [X / faceCount, Y / faceCount, Z / faceCount];
    }

    function faceDistance(referencePoint, x,y,z,i,j,k) {
        var X = referencePoint[0];
        var Y = referencePoint[1];
        var Z = referencePoint[2];
        var faceCount = i.length;
        var f, fx, fy, fz;
        var euclideanDistance = [];
        for(f = 0; f < faceCount; f++) {
            fx = (x[i[f]] + x[j[f]] + x[k[f]]) / 3;
            fy = (y[i[f]] + y[j[f]] + y[k[f]]) / 3;
            fz = (z[i[f]] + z[j[f]] + z[k[f]]) / 3;
            euclideanDistance.push(Math.sqrt(Math.pow((fx - X) / 67, 2) + Math.pow((fy - Y) / 172, 2) + Math.pow((fz - Z)/122, 2)));
        }
        return euclideanDistance;
    }

    function centroidDistance(x,y,z,i,j,k) {
        var centroid = faceCentroid(x,y,z,i,j,k);
        var distance = faceDistance(centroid, x,y,z,i,j,k);
        return distance;
    }

    function emphasizeColorWithMap(intensity) {
        var intensityDomain = d3.extent(intensity)
        var intensityScale = d3.scale.quantile().domain(intensityDomain).range(viridis); // viridis is the 256-length array of Viridis colors
        var rescaledIntensity = intensity.map(intensityScale)
        var rgbNew = rescaledIntensity.map(function(tuple) {
            return 'rgb(' + tuple[0] + ',' + tuple[1] + ',' + tuple[2] + ')'
        })
        return rgbNew;
    }

    var measure = centroidDistance(x, y, z, i, j, k)
    mock.data[0].facecolor = emphasizeColorWithMap(measure.map(function(i) {return (i)}));

Of course an ellipsoid shape is simplistic, I'm sure there are established ways for fitting an anatomy-identifying model to a specific brain.
image
image

@choldgraf
Copy link
Contributor Author

@monfera that's beautiful. clever work!

I showed these images to one of the devs at pysurfer, and they're interested in exploring using plotly as the backend for their 3D visualizations. pysurfer is a package for visualizing brain surfaces and plotting activity on top of them. Check out their repo here. It integrates pretty heavily with freesurfer which is sort of the gold standard for creating these surfaces in the first place.

They're using Mayavi right now which is pretty clunky, it'd be great if there were a faster / more stable / prettier plotting solution (e.g., these plots get pretty close). I'll probably open an issue there to discuss switching the backend to plotly and see what people think. Will keep you guys updated.

@ntfrgl
Copy link

ntfrgl commented Jul 5, 2016

This is a Plotly beginner's question. Please let me know if it fits better elsewhere.

Custom colours on a highly interactive trisurf rendering within the browser are exactly what I need, but for many applications including mine, a colour bar is essential for visual interpretation. Based on the documentation on trisurf, surface coloring and meshes, I guessed that something like

fig.data[0].update(colorbar=go.ColorBar(...))

would do the job, but this was ineffectual. What would be the right way? I would have expected it to be a parameter for create_trisurf().


Also, I find a regular oscillation with high frequency and high amplitude in my RAM consumption when rendering a big trisurf inside an ipython notebook within Chrome. Not so for Firefox. Is this a general issue with the underlying js libraries?

@monfera
Copy link

monfera commented Jul 5, 2016

@ntfrgl this PR and associated PRs indeed deal with mesh coloring. On the plotly.js side of things it involved an update that solved lighting and reflection issues that in the past turned the mesh show up in a subpar color or no color. Since color representation, annotation and data flow haven't changed, i.e. the possible causes seem unrelated, it would be better to document new issues (of course they can link to this or other PRs).

When creating the new issue, it's helpful if a minimal example is included or linked that demonstrates the problem. If the problems were shown via the Python API, then it's useful to reproduce it with the JS API directly as well, because this can suggest the location of the issue.

The utility of minimal problem reproductions also mean that it's useful to separate issues, e.g. one for the legend (with your example of what didn't work) and one for the memory fluctuation (maybe with a screenshot of Chrome vs Firefox). Easiest thing might be if there's a link to a demonstrating example.

A comment about memory fluctuations is that we started looking into some aspects of memory management, which have a speed impact as well, so any report like yours is very useful.

@ntfrgl
Copy link

ntfrgl commented Jul 5, 2016

Opened #517, commented in #487.

@choldgraf choldgraf deleted the custom_colors_trisurf branch July 26, 2016 21:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants