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

unlist tmp to match dist2land's output with Windows #25

Merged
merged 2 commits into from
Jun 23, 2023

Conversation

SESjo
Copy link
Contributor

@SESjo SESjo commented Jun 22, 2023

On Windows, results are stored in out variable, but never unlisted in a tmp variable, resulting into a column filled with 'lat' and 'lon' values because tmp is also created at the beginning of the function with these two values. So I just added tmp <- unlist(out) and that did the trick!

On Windows, results are stored in out variable, but never unlisted in a tmp variable resulting into a column filled with 'lat' and 'lon' value because tmp is also created at the beginning of the function with these two values.
unlist tmp to match dist2land's output with Windows
@MikkoVihtakari MikkoVihtakari merged commit 884a48f into MikkoVihtakari:master Jun 23, 2023
@MikkoVihtakari
Copy link
Owner

Thanks for the fix! I never tried parallel processing under Windows. I assume it works now?

Btw, I am currently rewriting the entire package to sf. Need to rewrite dist2land too once I'll get that far.

@SESjo
Copy link
Contributor Author

SESjo commented Jun 23, 2023

That's good to hear, your package is amazing! Yep, it works pretty well under Windows, I usually run things under Linux but this time I had to use Windows, otherwise I would never have caught this one :)

@MikkoVihtakari
Copy link
Owner

I updated dist2land now. It's in a separate sf-version branch which I will make main branch once the conversion is complete. I could considerably simplify the function due to s2 and sf. The function uses great circle distances on spherical Earth now. The downside is that the calculus could not be parallelised easily so the option to use multiple cores is gone and as a result the function is slower for large datasets than it used to be.

If you had the time, could you test the function and see how it works for you?

I found one bug. Lon/lat 0/0 yields land for some reason. Otherwise the distances look reasonable but I have not confirmed them by measuring on a map for instance.

The function is here and the branch here: https://github.com/mikkovihtakari/ggoceanmaps/tree/sf-version

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