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

WIP: Support for curvilinear coordinates #16

Merged
merged 6 commits into from Jan 24, 2020
Merged

Conversation

darwindarak
Copy link
Collaborator

It almost works!

θ = linspace(0,2π,10)
R = linspace(1,3, 10)
ζ = Complex128[r*exp(im*ϕ) for ϕ in θ, r in R]
x, y, z = real(ζ), imag(ζ), abs(ζ)

so z has the form

10x10 Array{Float64,2}:
 1.0  1.22222  1.44444  1.66667  1.88889  2.11111  2.33333  2.55556  2.77778  3.0
 1.0  1.22222  1.44444  1.66667  1.88889  2.11111  2.33333  2.55556  2.77778  3.0
 1.0  1.22222  1.44444  1.66667  1.88889  2.11111  2.33333  2.55556  2.77778  3.0
 1.0  1.22222  1.44444  1.66667  1.88889  2.11111  2.33333  2.55556  2.77778  3.0
 1.0  1.22222  1.44444  1.66667  1.88889  2.11111  2.33333  2.55556  2.77778  3.0
 1.0  1.22222  1.44444  1.66667  1.88889  2.11111  2.33333  2.55556  2.77778  3.0
 1.0  1.22222  1.44444  1.66667  1.88889  2.11111  2.33333  2.55556  2.77778  3.0
 1.0  1.22222  1.44444  1.66667  1.88889  2.11111  2.33333  2.55556  2.77778  3.0
 1.0  1.22222  1.44444  1.66667  1.88889  2.11111  2.33333  2.55556  2.77778  3.0
 1.0  1.22222  1.44444  1.66667  1.88889  2.11111  2.33333  2.55556  2.77778  3.0

which would look like straight lines if plotted in rectangular coordinates. But if the matrices x and y that contain the coordinate points are passed in, we get

circles

There are still a few kinks to work out though. For example:

θ = linspace(0,2π,100);
R = linspace(1,2, 50);
ζ = Complex128[r*exp(im*ϕ) for ϕ in θ, r in R];
x, y = real(ζ), imag(ζ);
z = imag.+ conj(1./ζ));

results in

cylinder

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling ee1173f on dd/curvilinear_coords into 2d5ca7d on master.

@tomasaschan
Copy link
Contributor

Very cool! I have no idea what the quirks are about, but I'm sure you'll find it =)

I have a couple of concerns about argument types for some of these, but that should be quite easy to fix. I'll comment on the relevant code lines separately.

src/Contour.jl Outdated
@@ -153,7 +152,7 @@ end
# Given the row and column indices of the lower left
# vertex, add the location where the contour level
# crosses the specified edge.
function interpolate{T<:FloatingPoint}(x, y, z::Matrix{T}, h::Number, xi::Int, yi::Int, edge::Uint8)
function interpolate{T<:FloatingPoint}(x::Vector{Float64}, y::Vector{Float64}, z::Matrix{T}, h::Number, xi::Int, yi::Int, edge::Uint8)
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really need to enforce Vector{Float64} here? I suggest AbstractVector{T}, or even just AbstractVector. I guess the point is to distinguish from the method that takes a matrix below, but since the only important type parameter is the dimension (1 vs 2), we should try to keep everything else as generic as possible.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That makes sense. I think the only requirement we need is for the type to have arithmetic functions defined for it.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling ef83904 on dd/curvilinear_coords into 2d5ca7d on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling e048006 on dd/curvilinear_coords into 2d5ca7d on master.

@darwindarak
Copy link
Collaborator Author

I think I fixed it, so now we can make plots like this:

cylinder

without having to do too many coordinate transforms.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 3b64d19 on dd/curvilinear_coords into 2d5ca7d on master.

@tomasaschan
Copy link
Contributor

Nice!

Still needs documentation, though, even though these examples are a good starting point - I'd need a mathematical description of how to create x and y to understand how to do it for whatever system I'm working in.

@darwindarak
Copy link
Collaborator Author

Yeah, definitely. I'll add the documentation as soon as I can. There are also a few edge cases I want to test out.

@SimonDanisch
Copy link
Member

@darwindarak, would be nice to merge this ;)

@darwindarak
Copy link
Collaborator Author

Just rebased on master branch and updated the syntax. @SimonDanisch I haven't used Julia in a while, do you mind taking a look to see if anything needs to be changed? Otherwise I'll merge once the tests go through.

@sjkelly sjkelly self-requested a review January 24, 2020 15:34
@sjkelly
Copy link
Member

sjkelly commented Jan 24, 2020

This will be helpful to my lab mates. I say lets merge and work out the bugs.

edit: The code looks good!

@darwindarak darwindarak merged commit b5c1779 into master Jan 24, 2020
@SimonDanisch SimonDanisch deleted the dd/curvilinear_coords branch January 24, 2020 22:33
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.

None yet

5 participants