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

[BUGFIX] Fix stuck sensor detection only applying to last column #177

Merged
merged 6 commits into from
Mar 11, 2024

Conversation

misi9170
Copy link
Collaborator

@misi9170 misi9170 commented Mar 5, 2024

Addresses issue raised in #175.

The stuck sensor indices were only being returned for the last column checked, as opposed to a union of the indices over all columns. This PR addresses that issue.

To do

  • Address fundamental issue and ensure that indices are returned for any stuck sensor (not just final column)
  • Check that examples still run as expected
  • Add test for this (can base on @DanielKerrigan 's example script)
  • to discuss: do we want to return a separate list of indices for each column provided, or simply a union of all indices that contain a stuck sensor fault in at least one column (the current expected behavior)? I decided to add an option to do this, defaulting to False, which when True will instead return a dictionary containing a numpy array of stuck sensor indices for each column passed.

@DanielKerrigan 's script below now produces the expected results

import pandas as pd
from flasc.turbine_analysis.find_sensor_faults import find_sensor_stuck_faults

df = pd.DataFrame({"a": [0, 0, 0, 1, 2, 3], "b": [4, 5, 6, 7, 7, 7]})

results_a = find_sensor_stuck_faults(df, columns=["a"], ti=0, plot_figures=False)
print("columns=['a']:", results_a)

results_b = find_sensor_stuck_faults(df, columns=["b"], ti=0, plot_figures=False)
print("columns=['b']:", results_b)

results_ab = find_sensor_stuck_faults(df, columns=["a", "b"], ti=0, plot_figures=False)
print("columns=['a', 'b']:", results_ab)

results_ba = find_sensor_stuck_faults(df, columns=["b", "a"], ti=0, plot_figures=False)
print("columns=['b', 'a']:", results_ba)

# Output:
# columns=['a']: [0 1 2]
# columns=['b']: [3 4 5]
# columns=['a', 'b']: [0 1 2 3 4 5]
# columns=['b', 'a']: [0 1 2 3 4 5]

@misi9170 misi9170 self-assigned this Mar 5, 2024
@misi9170 misi9170 added the bug Something isn't working label Mar 5, 2024
@misi9170 misi9170 force-pushed the bugfix/stuck-sensor-multiple-columns branch from e4c9e14 to e319a1f Compare March 5, 2024 04:03
@misi9170 misi9170 marked this pull request as draft March 5, 2024 04:04
@misi9170
Copy link
Collaborator Author

misi9170 commented Mar 5, 2024

Two of the examples (that I'm aware of) use find_stuck_sensor_faults(), called within ws_pow_filtering.py's filter_by_sensor_stuck_faults():

  • examples_artificial_data/01_raw_data_processing/00_filter_ws_power_curves.ipynb
  • examples_smarteole/03_filter_ws_power_curves.ipynb
    In these, find_stuck_sensor_faults() is being called separately for each turbine in a for loop; however, the turbine's wd and ws columns are checked simultaneously, in that order, so the bug meant that only the stuck ws indices were being returned. After the bugfix, there is a slight change (increase) to the data removed by the stuck sensor filter for examples_artificial_data/01_raw_data_processing/00_filter_ws_power_curves.ipynb; and there is no (or minimal) change to the data removed by the stuck sensor filter in examples_smarteole/03_filter_ws_power_curves.ipynb (presumably, there were few/no instances of stuck wd readings).
    Out of caution, I have rerun any subsequent notebooks after these examples so that they are consistent with any changes to the data made.

@misi9170 misi9170 marked this pull request as ready for review March 5, 2024 17:10
@paulf81 paulf81 self-requested a review March 11, 2024 20:15
Copy link
Collaborator

@paulf81 paulf81 left a comment

Choose a reason for hiding this comment

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

These changes look good @misi9170, and thank you for adding the tests

@misi9170
Copy link
Collaborator Author

Thanks @paulf81 . I realized I hadn't pushed the changes to the examples (not to the running code, just to the notebook output), so I'll do that now and then merge.

@misi9170 misi9170 merged commit 0c1fb64 into NREL:develop Mar 11, 2024
3 checks passed
@misi9170 misi9170 deleted the bugfix/stuck-sensor-multiple-columns branch March 11, 2024 20:45
@misi9170 misi9170 mentioned this pull request Apr 10, 2024
2 tasks
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

Successfully merging this pull request may close these issues.

2 participants