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

v0.2.5 skin is broken #38

Open
jbusecke opened this issue Jun 24, 2022 · 2 comments
Open

v0.2.5 skin is broken #38

jbusecke opened this issue Jun 24, 2022 · 2 comments
Labels
bug Something isn't working

Comments

@jbusecke
Copy link
Contributor

Sorry for the oversight. The fact that tests show as passed in the CI while they fail in the fortran code is really dicey. We (I especially since that has happened multiple times to me at this point) have to check each of the CI actions and confirm that pytest actually completed the test and did not crash.

Back to the problem at hand though. Unfortunately in #34 I forgot to reactivate the test for the skin xarray wrapper, which lead to an actually passing CI! I noticed the missing test parametrization in #35, but missed that the tests did not pass.

So it seems that threadsafe somehow broke the skin but not the noskin method but only in the case of certain chunking schemes. Very weird, ill investigate now.

@jbusecke jbusecke added the bug Something isn't working label Jun 24, 2022
@jbusecke
Copy link
Contributor Author

I have dug as deep as I understand in #39, but do not see a quick solution for this.

Should we actually remove the threadsafe from the skin wrapper for now? This would enable the code to work, but probably not be very useable.
But it might be more desirable than code that just crashes?

@paigem @rabernat

@jbusecke
Copy link
Contributor Author

We have decided to shelf this issue for now.
In #40 the threadsafe was removed from the skin logic. It now works but is still slow on a threaded scheduler.
We will focus on driving the research forwards and reevaluate afterwards if we actually require the skin functionality.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant