-
Notifications
You must be signed in to change notification settings - Fork 28
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
(Closes #2755) Bug fix for psyad failing to transform array assignment #2767
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2767 +/- ##
=======================================
Coverage 99.88% 99.88%
=======================================
Files 356 356
Lines 49479 49480 +1
=======================================
+ Hits 49421 49422 +1
Misses 58 58 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
This needs a 'science' review by @DrTVockerodtMO and then I'll put it up for proper code review. |
Will be happy to, just need to figure out how to test specific branches on our system and I'll have a look! |
@DrTVockerodtMO and I have jointly test this branch against some kernels and everything looks OK. This is therefore ready for review. It's not a big change. Could be one for @hiker, @sergisiso, @LonelyCat124 |
I've just set the integration tests going. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor comment issues, and the need to bring it up to master.
Somehow, some warnings related to the Dev Guide had made it onto master and were causing the link check to fail. I've fixed those here so that the CI is green. |
This should be ready for another look now @hiker. |
The doc build is still failing but that's because the link to the OpenCL standard is returning a 403. If I visit the URL I get a check that I am a human so presumably this is what the problem is. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All good. I've triggered the CI (because of changes to array_mixin), and will continue to merge if this is successful.
Hi @hiker, note that the integration-test failure for NEMO5 is expected (@sergisiso says he's broken something). |
Assuming that the NEMO failures are known (as indicated by Andy), and the link check is also unrelated, I progress with merging. |
A small change that (pleasingly) only involved me updating some code to remove an old restriction. @DrTVockerodtMO please could you try this branch and let me know whether it solves your problem?