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

diff_order seems to be hard coded to be diff order of 2 #120

Open
pdurham2 opened this issue Oct 20, 2022 · 2 comments
Open

diff_order seems to be hard coded to be diff order of 2 #120

pdurham2 opened this issue Oct 20, 2022 · 2 comments
Labels
bug Something isn't working

Comments

@pdurham2
Copy link
Contributor

When diff_order is applied in lad_filtering.py, the value passed to np.diff is fixed as 2. Is this intended or should diff_order be passed instead?

if diff_order:
  actual_previous_per_diff = [interpolated_actual_previous[-1]] \
      if diff_order == 1 else [interpolated_actual_previous[-1], np.diff(interpolated_actual_previous)[0]]
  seq_tail = interpolated_actual_previous + [interpolated_actual]
  interpolated_actual = np.diff(seq_tail, 2)[-1]
@pdurham2 pdurham2 changed the title diff_order doesnt seems to be hard coded to be diff order of 2 diff_order seems to be hard coded to be diff order of 2 Oct 20, 2022
@sayanchk
Copy link
Collaborator

sayanchk commented Oct 22, 2022

@pdurham2 I think your observation is correct. Based on the logic for this part of the code, this should be interpolated_actual = np.diff(seq_tail, diff_order)[-1].

Please feel free to make the change and add me as a reviewer. We should also test the change by training and scoring the filtering model using an example dataset (you can use any dataset from https://github.com/zillow/luminaire/tree/master/luminaire/tests/datasets) to check if everything is working as expected.

@pdurham2
Copy link
Contributor Author

@sayanchk sounds good, will do that soon - thanks for confirming!

@sayanchk sayanchk added the bug Something isn't working label Oct 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants