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

Add Blocked timestepping support to PyRevolve #55

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

speglich
Copy link
Contributor

Closes #54

@navjotk
Copy link
Member

navjotk commented Apr 28, 2022

Can you please clean up the commit history on this a bit? e.g. I see some commits undoing things done by previous commits.

@speglich
Copy link
Contributor Author

Can you please clean up the commit history on this a bit? e.g. I see some commits undoing things done by previous commits.

Done.

@navjotk
Copy link
Member

navjotk commented May 10, 2022

This is showing a merge conflict after merging the previous PR. Could you please do a rebase on current master?

@speglich
Copy link
Contributor Author

This is showing a merge conflict after merging the previous PR. Could you please do a rebase on current master?

Done.

@speglich
Copy link
Contributor Author

Help me to remember... there is something missing @navjotk?

@@ -123,7 +123,11 @@ def apply(self, **kwargs):
t_end = kwargs['t_end']
assert(t_start <= t_end)
if self.direction == 1:
self.u[:] = self.u[:] + self.direction * abs(t_start - t_end)
for t in range(t_start, t_end):
Copy link
Member

Choose a reason for hiding this comment

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

What is happening here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We added a new dimension to the u array. This dimension is responsible to save the block_size time steps. That before are not saved because isn't necessary. This loop is responsible to write each time step in the right position.

Copy link
Member

@navjotk navjotk left a comment

Choose a reason for hiding this comment

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

There is a misunderstanding of how the IncOperator works - please fix that.

@@ -123,7 +123,11 @@ def apply(self, **kwargs):
t_end = kwargs['t_end']
assert(t_start <= t_end)
if self.direction == 1:
self.u[:] = self.u[:] + self.direction * abs(t_start - t_end)
for t in range(t_start, t_end):
idx = t % np.shape(self.u)[0]
Copy link
Member

Choose a reason for hiding this comment

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

There must be a better way of getting block_size

self.u[:] = self.u[:] + self.direction * abs(t_start - t_end)
for t in range(t_start, t_end):
idx = t % np.shape(self.u)[0]
past_idx = (t-1) % np.shape(self.u)[0]
Copy link
Member

Choose a reason for hiding this comment

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

You're assuming that the operator moves one time step at a time, when this has is supposed to go from t_start to t_end.

for t in range(t_start, t_end):
idx = t % np.shape(self.u)[0]
past_idx = (t-1) % np.shape(self.u)[0]
self.u[idx][:] = self.u[past_idx][:] + self.direction * 1
Copy link
Member

Choose a reason for hiding this comment

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

Again, assuming one time step at a time. Not true.

else:
self.v[:] = (self.u[:]*(-1) + 1)
idx = (t_start) % np.shape(self.u)[0]
Copy link
Member

Choose a reason for hiding this comment

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

You have to fill in all elements from t_start to t_end.

@navjotk
Copy link
Member

navjotk commented Oct 18, 2022

@speglich any progress here?

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.

Add Support to Blocked Timestepping
2 participants