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.

Sign up for GitHub

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

Jump to bottom

Bug fix: various small corrections #86

Merged
merged 15 commits into from
Jun 30, 2023
Merged

Bug fix: various small corrections #86

merged 15 commits into from
Jun 30, 2023

Conversation

Bartdoekemeijer
Copy link
Collaborator

This PR is not yet ready to be merged.

Feature or improvement description
Various bug fixes throughout the code.

Related issue, if one exists
I found various issues running FLASC for wind farm SCADA analyses.

Impacted areas of the software
Various

Additional supporting information

Test results, if applicable

paulf81
paulf81 approved these changes May 30, 2023
View reviewed changes
Copy link
Collaborator

@paulf81 paulf81 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I had some feedback at WESC there was a little trouble setting up FLASC so perhaps related to these issues you identify @Bartdoekemeijer , maybe we can call this a hot fix, pull to main and do some 0.01 increment in version?

@Bartdoekemeijer
Copy link
Collaborator Author

Yes let's do that! There might be a couple more bugs that I haven't found yet though. I'm working on FLASC a lot over the next 2 weeks. Is it OK if we keep this PR open until then? That way I can add on as I find more bugs.

@paulf81
Copy link
Collaborator

paulf81 commented May 31, 2023

Yes let's do that! There might be a couple more bugs that I haven't found yet though. I'm working on FLASC a lot over the next 2 weeks. Is it OK if we keep this PR open until then? That way I can add on as I find more bugs.

Sounds good to me! I will also be tinkering (polars and database stuff), I just caught my polars pull up to yours and will try to keep up

@Bartdoekemeijer
Copy link
Collaborator Author

Sorry I had made an absolute mess mixing up some PRs here. Restored it with a force push.

@Bartdoekemeijer Bartdoekemeijer mentioned this pull request Jun 15, 2023
Closed
@Bartdoekemeijer
Copy link
Collaborator Author

In the most recent commit, I vectorized some code that was previously in a for loop. This helped speed up the part where we calculate the histogram/median power curve based on the data. Additionally, I have reduced the calls to this function to save time, and only call it when it's actually needed. The ws_pow_filtering code now runs about a factor 5 times faster on my machine.

@Bartdoekemeijer Bartdoekemeijer mentioned this pull request Jun 22, 2023
Closed
@Bartdoekemeijer
Copy link
Collaborator Author

Bartdoekemeijer commented Jun 27, 2023 •
edited
Loading

OK to merge into main if you ask me! Any future bug fixes, I'll open a new PR for.

@Bartdoekemeijer
Copy link
Collaborator Author

Just committed the PyPI bug fix from @rafmudaf.

@paulf81
Copy link
Collaborator

paulf81 commented Jun 30, 2023

@rafmudaf do you approve merging? If so you can either merge or let me know and I'll do it, thanks!!

@rafmudaf
Copy link
Collaborator

@paulf81 Yes, from the perspective of the setup.py infrastructure, this is good to go.

@paulf81 paulf81 merged commit b058c78 into NREL:main Jun 30, 2023
@paulf81
Copy link
Collaborator

paulf81 commented Jun 30, 2023

Merged!

This was referenced Jul 3, 2023
Closed
Merged
Bartdoekemeijer added a commit that referenced this pull request Jul 3, 2023
@Bartdoekemeijer @paulf81 @rafmudaf
Sync: Merge v1.3.1 bug fixes into develop (#96)
d3b2deb 
* Merge in changes from `main` branch (v1.3 -> v1.3.1) into `develop`, notably:

- Feature: Add PyPI integration by @Bartdoekemeijer in #87
- Bug fix: various small corrections by @Bartdoekemeijer in #86

---------

Co-authored-by: paulf81 <paul.fleming@nrel.gov>
Co-authored-by: Rafael M Mudafort <rafael.mudafort@nrel.gov>
This was referenced Jul 3, 2023
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rafmudaf rafmudaf rafmudaf left review comments

@paulf81 paulf81 paulf81 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Bartdoekemeijer @paulf81 @rafmudaf