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

Use concrete number of grid regions #9

Merged
merged 3 commits into from
Nov 23, 2024
Merged

Conversation

pjaap
Copy link
Member

@pjaap pjaap commented Nov 18, 2024

Is there any reason why this had the minimal number 5 before?

@j-fu
Copy link
Member

j-fu commented Nov 18, 2024

Well, what is happening n=1, 2,3 ? If it looks reasonable it would be ok to remove the minimum. But the result should be checked at least with gridplot with Makie, PyPlot and PlutoVista. Not sure if all these codes work e.g. with n=1.

@pjaap
Copy link
Member Author

pjaap commented Nov 18, 2024

The GLMakie output is here: WIAS-PDELib/GridVisualize.jl#38
I'll check the other Plotters as well

@pjaap
Copy link
Member Author

pjaap commented Nov 18, 2024

PlutoVista seems fine and shows now only 4 boundary segments. I am not familiar with it: Doesn't it show cell regions?

image

@pjaap
Copy link
Member Author

pjaap commented Nov 18, 2024

But PyPlot crashes indeed. I will take a look there.

@pjaap
Copy link
Member Author

pjaap commented Nov 18, 2024

PyPlot works with a few fixes: WIAS-PDELib/GridVisualize.jl#39

There was a wrong ncellregions instead of nbfaceregions and PyPlot.Colobar only works for n>1. I added a safety check to increase the number to 2 in case this is 1.

@j-fu
Copy link
Member

j-fu commented Nov 23, 2024

Ok let us handle the problematic cases in the particular backends.

... we changed Colors.RGB() to rgbcolor() to remove piracy, so breaking
@j-fu j-fu merged commit d4f67f6 into main Nov 23, 2024
11 checks passed
@j-fu j-fu deleted the feature/concrete-region-numbers branch November 23, 2024 22:17
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.

2 participants