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 maxtmax argument #159

Merged
merged 3 commits into from
Oct 25, 2019
Merged

Add maxtmax argument #159

merged 3 commits into from
Oct 25, 2019

Conversation

rubencalje
Copy link
Collaborator

No description provided.

Comment on lines 228 to 233

def get_block(self, p, dt, tmin, tmax):
"""Internal method to get the block-response from the respnse function"""
maxtmax = (tmax-tmin)/pd.to_timedelta(1,'d')
b = self.rfunc.block(p, dt, maxtmax=maxtmax)
return b
Copy link
Member

Choose a reason for hiding this comment

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

I think it would be better to rename this to "get_block_response" or simply "block". And then maybe we refactor step to step_response as well while we are at it.

Also I think refund.block requires the cutoff?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is in stressmodels.py, so there is no step here. I use get_block() instead of block() to show the difference between the block-method in rfunc.py. The rfunc gets the cutoff from self, as it was passed earlier already from the stressmodel.

@@ -851,7 +857,7 @@ def get_tmax(self, p, cutoff=None):
def gain(self, p):
return 1.

def step(self, p, dt=1, cutoff=None):
t = self.get_t(p, dt, cutoff)
def step(self, p, dt=1, cutoff=None, maxtmax=None):
Copy link
Member

Choose a reason for hiding this comment

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

Mark this as an internal method just like the block?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is another file (rfunc.py instead of stressmodels.py)


def get_block(self, p, dt, tmin, tmax):
"""Internal method to get the block-response from the respnse function"""
maxtmax = (tmax-tmin)/pd.to_timedelta(1,'d')
Copy link
Member

Choose a reason for hiding this comment

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

This involves another hardcoding of daily in Pastas.... just noting.

@raoulcollenteur
Copy link
Member

Why is this change necessary actually? #156 #159

Replaces FourParam-definition in notebook to the one in pastas
@rubencalje
Copy link
Collaborator Author

Why is this change necessary actually? #156 #159

Yes, because of issue #156

@rubencalje
Copy link
Collaborator Author

Strange error in Travis CI: RuntimeError: Kernel didn't respond in 60 seconds

In the pr-check of Travis this build is passed. I think this pr is ready to be merged.

@raoulcollenteur
Copy link
Member

I restarted the build on Travis, let's see what happens? This should work before merging..

@raoulcollenteur raoulcollenteur merged commit 8678a2e into dev Oct 25, 2019
@raoulcollenteur raoulcollenteur deleted the maxtmax branch October 25, 2019 09:11
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.

None yet

2 participants