Skip to content
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

Change mask color for nan/inf values to light grey for contour plots #180

Merged
merged 2 commits into from
Nov 21, 2024

Conversation

mjprilliman
Copy link
Collaborator

-Rather than drawing nonfinite results as the minimum color in contour plots, draw them as light grey
-Start from the minimum value for contour layer plotting rather than min value + 1; previously, contour made min color the background then counted from the next layer
-Light grey preset is now background color before any layers are drawn
-Useful in parametrics with Nan or inf values in outputs, such as payback period or internal rate of return

with min rather than min+1 to not draw entire background as min
@mjprilliman mjprilliman requested a review from sjanzou November 18, 2024 19:48
@mjprilliman mjprilliman self-assigned this Nov 18, 2024
@mjprilliman
Copy link
Collaborator Author

test_para.zip

Copy link
Collaborator

@sjanzou sjanzou left a comment

Choose a reason for hiding this comment

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

On MacOS 14.6.1, the test file parametrics are displayed nicely:
image

However, on the "Heat map" results tab, the default color map is jet and shows the NaN values as (same for all colormap selections)
image

The confusing issue is that on the "Heat map" page, the NaN color is the same as the zero color
image

with data comparison
image

Regardless, your pull request is a great improvement!

@@ -250,16 +261,17 @@ void wxPLContourPlot::Draw(wxPLOutputDevice &dc, const wxPLDeviceMapping &map) {
dc.NoPen();
// background set to min
// assume RebuildMask has been called
wxColor bgc(m_cmap->ColourForValue(m_zMin));
//wxColor bgc(m_cmap->ColourForValue(m_zMin));
wxColor bgc(*wxLIGHT_GREY);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe this should be a defined value like CONTOUR_BG or COUNTOUR_NAN_COLOR for future maintenance - what do you think?

@mjprilliman
Copy link
Collaborator Author

On MacOS 14.6.1, the test file parametrics are displayed nicely: image

However, on the "Heat map" results tab, the default color map is jet and shows the NaN values as (same for all colormap selections) image

The confusing issue is that on the "Heat map" page, the NaN color is the same as the zero color image

with data comparison image

Regardless, your pull request is a great improvement!

This is happening in plcolourmap.cpp line 162. Essentially, the wxDVDMapCtrl is passing NaNs to the ColourForValue() function, where the wxPLContourPlot class is masking nonfinite values and essentially skipping them in the ColourForValue process, just keeping the gray background color. An easy fix would be to make the Non-finite color for the heatmaps the light grey instead of black, if you think that makes sense.

image

@mjprilliman
Copy link
Collaborator Author

image

From latest commit. Also check implementation of wxCONTOUR_BG definition in plcolourmap.h (included in both heat map generation files).

@mjprilliman mjprilliman requested a review from sjanzou November 19, 2024 17:27
Copy link
Collaborator

@sjanzou sjanzou left a comment

Choose a reason for hiding this comment

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

Looks good on the mac!

@sjanzou sjanzou merged commit 824c11c into develop Nov 21, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants