Parse a windIO yaml input file and instantiate the turbine structs #197
Conversation
…y, add some comments for clarity, remove some empty lines etc.
…etter readability
…from alphabetic order -> the order they appear within the schema to help with compilation error due to ordering
faisal-bhuiyan
changed the title
|
I think it looks good, just need to clean up all the Correctness and CppCheck comments |
| - name: Cache install yaml-cpp | ||
| id: cache-yaml-cpp | ||
| uses: actions/cache@v3 | ||
| with: | ||
| path: ${{ github.workspace }}/spack | ||
| key: linux-yaml-cpp | ||
| - name: Install yaml-cpp | ||
| if: steps.cache-yaml-cpp.outputs.cache-hit != 'true' | ||
| run: | | ||
| source spack/share/spack/setup-env.sh | ||
| spack install yaml-cpp |
There was a problem hiding this comment.
Once you push this in, I'm thinking of unifying it with the action above so that we only have one "Build Dependencies" step/cache. Any thoughts/objections?
There was a problem hiding this comment.
No objections from me, feel free to take care of it when you work on the CI pipeline as discussed.
|
|
||
| #include "src/utilities/scripts/windio_mapped_structs.hpp" | ||
|
|
||
| namespace openturbine::restruct_poc::tests { |
There was a problem hiding this comment.
If you want to avoid the need for filesystem logic, you can store the contents of the yaml file in a string (make a test_yaml_parser.hpp with a variable std::string IEA15File = "..."). Then YAML_::Load(IEA15File) should parse it.
There was a problem hiding this comment.
I gave it a shot -- looks like the string literal is too long for the liking of the compiler. I'll defer to you on the best course of action here.
There was a problem hiding this comment.
No problem. I might play with it if I feel froggy, but it's good to go for now.
|
@ddement Could you take a look at the errors from clang-tidy here? They are looking a little arbitrary to me since I did not see these errors the last time and I don't see them on my macOS dev environment. https://github.com/Exawind/openturbine/actions/runs/10375210534/job/28724356604?pr=197 PS: I am proposing we change the platform for clang-tidy and cppcheck to macOS. Any thoughts, @deslaughter? |
|
@faisal-bhuiyan It is failing now because the latest main branch has now enabled enforcing clang-tidy in header files, so you can see that your new errors are from the windio-mapped-struct header. If you need me to, I can take a look at getting that file passing clang-tidy on Linux and push up the necessary changes. |
|
@ddement Is this good to get merged now? |
@faisal-bhuiyan Looks good to me. Feel free to merge and I'll get to work on the CI stuff we discussed |
Completes #175.
This follows up on the work performed in #199 by adding the following
parse()methods to each of the mappedStructs