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

Static yaw misalignment analysis method #249

Merged
merged 31 commits into from
Aug 31, 2023

Conversation

ejsimley
Copy link
Collaborator

This pull request contributes a new analysis method for estimating static yaw misalignment for individual turbines using SCADA data. The PR includes the following changes and additions:

  • The yaw_misalignment analysis module which contains the StaticYawMisalignment class for performing the yaw misalignment assessment using a PlantData object
  • A new plot_yaw_misalignment function in the plot utils module, which is used by the StaticYawMisalignment class to help visualize yaw misalignment
  • The notebook "07_static_yaw_misalignment.ipynb", which illustrates the yaw misalignment method applied to the LHB data
  • Updates to the sphinx documentation to include the yaw misalignment analysis method
  • Yaw misalignment regressions tests
  • Updated analysis requirements in metadata.py, including the addition of asset data requirements for the WakeLosses and StaticYaw Misalignment analysis methods

This pull request closes issue #230

@ejsimley ejsimley requested a review from RHammond2 July 12, 2023 19:53
Copy link
Collaborator

@RHammond2 RHammond2 left a comment

Choose a reason for hiding this comment

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

This is really good, thanks for yet another analysis class, @ejsimley! I've made a few small comments and amendments (only for really minor points about speedups via NumPy, or lower-level setup pieces that were missing), but nothing that fundamentally changes the work you've done.

I'll look into the failing tests a little bit more and the notebook commentary in more detail in another comment, but wanted to get you feedback on the code contributions first.

openoa/analysis/yaw_misalignment.py Outdated Show resolved Hide resolved
openoa/analysis/yaw_misalignment.py Outdated Show resolved Hide resolved
openoa/analysis/yaw_misalignment.py Outdated Show resolved Hide resolved
openoa/analysis/yaw_misalignment.py Show resolved Hide resolved
openoa/analysis/yaw_misalignment.py Show resolved Hide resolved
openoa/utils/plot.py Show resolved Hide resolved
openoa/utils/plot.py Show resolved Hide resolved
openoa/utils/plot.py Outdated Show resolved Hide resolved
openoa/utils/plot.py Outdated Show resolved Hide resolved
openoa/utils/plot.py Outdated Show resolved Hide resolved
@RHammond2
Copy link
Collaborator

@ejsimley, below I've made some comments on the new example notebook. Overall, it's a really great demonstration of the capabilities, nice work!

Cell 5

The plots should be generated by `openoa.utils.plot.plot_by_id(project.scada, "asset_id", "WMET_HorWdSpd", "WROT_BlPthAngVal"). However, I realize this also leaves room for improvement in that method for the actual axis labesl being additional input, in place of relying on the variable names themselves, and so I just created Issue #251.

Yaw misalignment detection without uncertainty quantification

I'd probably just have all "(UQ=False) instead of "(UQ=False)"

Cell 10

Looks like the empty axes are still being shown, so it'd be good to have them be removed if they're not in use. Let me know if you want any help with that.

Is your suggestion of power-weighted misalignment in the works? That sounds pretty interesting as an alternative, and if not, could be slightly misleading to the reader.

Conclusion

Should there be a sentence or two to end the notebook on what do with any of this? It feels like it ends abruptly

@ejsimley
Copy link
Collaborator Author

ejsimley commented Aug 9, 2023

Hi @RHammond2, thanks for the careful review and for helping to improve the code! I addressed all of the comments except for the ones that we discussed adding as separate issues. The unaddressed comments are:

  • yaw_misalignment.py: It seems like we should have an input validation method similar to the _analysis_validators.validate_UQ_inputs() for the run method inputs. I can make a separate PR to address this more effectively because I think much of the other analysis methods' inputs should be inputs to run() and not the class itself.
  • plot.py: I think it might be best to have this method in the utils subpackage, but I'm not totally sure where it belongs in there.
  • The plots should be generated by openoa.utils.plot.plot_by_id(project.scada, "asset_id", "WMET_HorWdSpd", "WROT_BlPthAngVal"). However, I realize this also leaves room for improvement in that method for the actual axis labesl being additional input, in place of relying on the variable names themselves, and so I just created Issue Finish cleaning up the plotting library #251

Let me know if you'd like me to work on any of those, or if you have any more comments on this PR.

Also, there are still a couple minor differences causing the tests to fail. I'm not sure what is the best way to address this.

@RHammond2
Copy link
Collaborator

Thanks @ejsimley! There seems to be one missing update, which is the only unresolved comment, otherwise this is ready to go from a code perspective.

As for the failing test, it seems that it's back to just the one invalid result in test_yaw_misaliginment_with_UQ for the expected_yaw_mis_results_95ci_ws values. I'm not too sure why this would be the case because they pass on my end still. Do they work on yours by any chance? I looked into the installed dependency versions previously, and didn't come up with anything on that front like we had previously. I wonder if we should tag Jordan in at this point, though I'll be out for the next week and can think a little more about it when I'm back.

@ejsimley
Copy link
Collaborator Author

Hi @RHammond2, thanks for reviewing the changes! See my response about the unresolved comment, which I think is already incorporated.

Regarding the tests, they all pass on my laptop as well. I don't have any ideas about how to address the lone test failure on the CI pipeline (other than further reducing the test precision), so we should discuss how to proceed.

@codecov-commenter
Copy link

Codecov Report

Patch coverage: 57.28% and project coverage change: -0.44% ⚠️

Comparison is base (c7f3414) 63.32% compared to head (91048d4) 62.88%.

Additional details and impacted files
@@              Coverage Diff               @@
##           develop_v3     #249      +/-   ##
==============================================
- Coverage       63.32%   62.88%   -0.44%     
==============================================
  Files              28       29       +1     
  Lines            3910     4209     +299     
==============================================
+ Hits             2476     2647     +171     
- Misses           1434     1562     +128     
Files Changed Coverage Δ
openoa/analysis/wake_losses.py 86.85% <ø> (ø)
openoa/utils/plot.py 4.81% <0.00%> (-0.71%) ⬇️
openoa/analysis/yaw_misalignment.py 84.77% <84.77%> (ø)
openoa/__init__.py 100.00% <100.00%> (ø)
openoa/analysis/__init__.py 100.00% <100.00%> (ø)
openoa/schema/metadata.py 97.27% <100.00%> (+0.01%) ⬆️
openoa/utils/filters.py 94.84% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@RHammond2 RHammond2 merged commit 8d12083 into NREL:develop_v3 Aug 31, 2023
2 checks passed
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.

3 participants