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
compiler: Patch iteration space scheduling #1898
Conversation
884a13d
to
bc94a6f
Compare
def test_issue_1897(self): | ||
grid = Grid(shape=(11, 11, 11)) | ||
|
||
v = VectorTimeFunction(name='v', grid=grid, time_order=1, space_order=4) |
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.
Orders can be dropped..space_order, time_order
Codecov Report
@@ Coverage Diff @@
## master #1898 +/- ##
==========================================
- Coverage 89.63% 89.63% -0.01%
==========================================
Files 210 210
Lines 35265 35272 +7
Branches 5319 5319
==========================================
+ Hits 31611 31616 +5
- Misses 3161 3162 +1
- Partials 493 494 +1
Continue to review full report at Codecov.
|
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.
Should a GPU test be added as well? Or redundant? It shouldn't;t fail I guess, but since John spotted a similar behaviour with tile(...) just saying...
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.
Wow that is a fast fix! Much impressed.
@@ -84,7 +84,7 @@ class Schedule(QueueStateful): | |||
|
|||
@timed_pass(name='schedule') | |||
def process(self, clusters): | |||
return self._process_fdta(clusters, 1) | |||
return self._process_fatd(clusters, 1) |
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.
Was really just a typo? 😮
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.
not sure if you are kidding? :-p
First Divide then apply
First apply then divide
It is such as small and nice fix though...like a typo
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.
Not kidding was just wondering if was intentional to have fdta
or if was meant to be fatd
from the beginning but wasn't noticed. anyway all good
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.
fatd
leads to a fundamental change in the algorithm behavior, so no, it was not a typo but rather an actual bug
No description provided.