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

Add ALM reg tests #1389

Merged
merged 4 commits into from
Dec 10, 2024
Merged

Add ALM reg tests #1389

merged 4 commits into from
Dec 10, 2024

Conversation

marchdf
Copy link
Contributor

@marchdf marchdf commented Dec 5, 2024

Summary

Add an ALM openfast reg tests. It's time. As much as we've been resisting this... It's critical enough and I recently broke something (#1385) that would have been caught if we had a reg test.

Pull request type

Please check the type of change introduced:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

Checklist

The following is included:

  • new unit-test(s)
  • new regression test(s)
  • documentation for new capability

This PR was tested by running:

  • the unit tests
    • on GPU
    • on CPU
  • the regression tests
    • on GPU
    • on CPU

@marchdf marchdf force-pushed the add-alm-regtest branch 2 times, most recently from 20d4a91 to d6866b9 Compare December 6, 2024 18:27
@marchdf
Copy link
Contributor Author

marchdf commented Dec 6, 2024

So far I have this. Red = T0, first run, green = T1, first run, blue = T0, restart run, orange = T1, restart run. They all do the same thing, which is expected. And blue and orange start later (since they are a restart)

turbines.pdf

@marchdf marchdf closed this Dec 6, 2024
@marchdf marchdf reopened this Dec 6, 2024
@marchdf
Copy link
Contributor Author

marchdf commented Dec 6, 2024

And with this restart case I can confirm the breakage I saw in #1385 and that breakage is not in 3.2.0.

@marchdf marchdf marked this pull request as ready for review December 6, 2024 19:32
@marchdf marchdf changed the title Add an ALM reg test Add ALM reg tests Dec 6, 2024
test/CMakeLists.txt Outdated Show resolved Hide resolved
Copy link
Contributor

@lawrenceccheung lawrenceccheung left a comment

Choose a reason for hiding this comment

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

Thanks @marchdf, this is great and something that has been sorely needed.

I'm adding in a note that the input files were generated with the notebook https://gist.github.com/lawrenceccheung/a775e8eb5162e906da2a75ce67169139, in case we want to generate other reg test type problems in the future.

Also, we should adopt a similar process of downloading the NREL5MW files from OpenFAST/r-test for the benchmark problems. Or least, noting which commit they were downloaded from and documenting which changes to the openfast model were necessary for the benchmark problems to run. I know it's for a separate problem, but we should be handling them consistently (Tagging @ndevelder on this).

Lawrence

amr-wind/wind_energy/actuator/Actuator.cpp Outdated Show resolved Hide resolved
@jrood-nrel jrood-nrel self-requested a review December 9, 2024 21:14
@lawrenceccheung lawrenceccheung self-requested a review December 10, 2024 05:13
@marchdf marchdf enabled auto-merge (squash) December 10, 2024 19:38
@marchdf marchdf merged commit 6acdd39 into Exawind:main Dec 10, 2024
15 checks passed
@marchdf marchdf deleted the add-alm-regtest branch December 10, 2024 20:19
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