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

Estimated tmax (way) too large for FourParam response function #209

Open
mbakker7 opened this issue Jul 8, 2020 · 4 comments
Open

Estimated tmax (way) too large for FourParam response function #209

mbakker7 opened this issue Jul 8, 2020 · 4 comments
Assignees
Labels
enhancement Indicates improvement of existing features priority 1 normal, deal with in the foreseeable future
Milestone

Comments

@mbakker7
Copy link
Collaborator

mbakker7 commented Jul 8, 2020

tmax for the 4-parameter function needs more testing. It goes wrong, for example:

fourp = ps.FourParam()
fp = fourp.step([2, 1, 50, 100], dt=1, cutoff=0.95)
@mbakker7 mbakker7 added the bug Indicates an unintended behavior or coding error label Jul 8, 2020
@raoulcollenteur
Copy link
Member

Just to add: this means that tmax is too large, but it won't negatively affect models using this response function. You just convolve with a very long block response function.

@mbakker7
Copy link
Collaborator Author

mbakker7 commented Jul 9, 2020 via email

@mbakker7 mbakker7 changed the title tmax not always computed correcty for 4-parameter function tmax sometimes estimated (way) too large for 4-parameter function Jul 21, 2020
@raoulcollenteur raoulcollenteur added this to the 1.0 milestone Nov 13, 2020
@raoulcollenteur raoulcollenteur added enhancement Indicates improvement of existing features and removed bug Indicates an unintended behavior or coding error labels Dec 1, 2021
@dbrakenhoff dbrakenhoff removed this from the 1.0: Arrabiata milestone Aug 16, 2022
@raoulcollenteur raoulcollenteur changed the title tmax sometimes estimated (way) too large for 4-parameter function estimated tmax (way) too large for FourParam response function Jan 19, 2023
@raoulcollenteur
Copy link
Member

For future reference, here some code to test

cutoff = 0.999
fourp = ps.FourParam()
fp = fourp.step([2, 1, 50, 100], dt=1, cutoff=cutoff)
print(len(fp[fp<cutoff]))

returns 525 days. So after 525 days 99.9% of the response has passed. The length of fp is almost 1e4 days though.

So this issue is still relevant.

@raoulcollenteur raoulcollenteur added this to the 1.1 milestone Jan 19, 2023
@martinvonk martinvonk changed the title estimated tmax (way) too large for FourParam response function [ENHANCEMENT] estimated tmax (way) too large for FourParam response function Feb 27, 2023
@martinvonk martinvonk added the priority 1 normal, deal with in the foreseeable future label Mar 7, 2023
@mbakker7
Copy link
Collaborator Author

It may be possible to determine the cutoff on the fly when stepping through time in computing the step response.

@martinvonk martinvonk modified the milestones: 1.1, 1.2 Aug 3, 2023
@raoulcollenteur raoulcollenteur modified the milestones: 1.2, 1.3 Aug 11, 2023
@raoulcollenteur raoulcollenteur modified the milestones: 1.3, 1.4 Nov 28, 2023
@raoulcollenteur raoulcollenteur modified the milestones: 1.4, 1.5 Feb 16, 2024
@raoulcollenteur raoulcollenteur changed the title [ENHANCEMENT] estimated tmax (way) too large for FourParam response function Estimated tmax (way) too large for FourParam response function Apr 9, 2024
@raoulcollenteur raoulcollenteur modified the milestones: 1.5, 1.6 Apr 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Indicates improvement of existing features priority 1 normal, deal with in the foreseeable future
Projects
None yet
Development

No branches or pull requests

4 participants