-
Notifications
You must be signed in to change notification settings - Fork 465
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
NWTC_Lib: Adding Yaml and VTK to library (moved from SD and AD) #1333
Conversation
Do we need to include |
So far I've put the modules outside of the "library module", but inside the library. |
@bjonkman do you have an opinion on whether these should be integrated into the library module? |
I like having these routines contained in the NWTC_Library directory. I could go either way on having them in the NWTC_Library module. If they were linking with or using other (external) modules, I would say to just keep them separated. Do you have plans to link with any other standard yaml or vtk libraries in the future? (I haven't looked what kind of standard libraries there are, though I've used C/C++ versions in other projects.) What about the existing One comment about the YAML Write Array 2D routines: it seems like they have optional arguments to output as json format. I'd be inclined to make separate subroutines for the json formats so that the routines always write yaml format, as their names indicate. |
Thanks Bonnie for the feedback. I think it will make sense to add the I agree that it's nice to put the JSON routines separately. I think it might make sense to put this into JSON.f90 (comparing the YAML.f90 and JSON.f90 would then be easy, as I expect some similar code between the two. I'll try to do that this afternoon. |
@ebranlard: FYI, it is possible that the ModMesh.f90 routines that write meshes to VTK may use those routines, so moving them into VTK.f90 might require putting module VTK in module NWTC_Library. |
Hi @bjonkman , yes, I'm currently noticing that. We could:
|
I've put the
I've done explicit imports at the beginning of these modules, let me know if you don't like that. I like it because it makes it clearer what the module relies on, see here: https://github.com/ebranlard/openfast/blob/f/nwtc-io-libs/modules/nwtc-library/src/JSON.f90#L21 If you have some quick feedback, I'm happy to include it. Otherwise, I'd like to move on, and let you free to rearrange this in future pull requests. |
a1210a5
to
87d8cc9
Compare
This pull request is ready to be merged
Feature or improvement description
This pull request:
I believe these modules fit within the "NWTC library" as they deal with generic IO operations.
Both AeroDyn and WakeDynamics have dependencies on VTK.f90.
Only SubDyn depends on Yaml.f90 for now, but I think it would be valuable to write the BeamDyn summary file in Yaml format, and the HydroDyn linearized matrices in that format too, so that their output can more easily be parsed (it's human and machine readable).
I'm happy to hear from @bjonkman as well if these changes are fine.
Impacted areas of the software
NWTC lib
Test results, if applicable
No change of tests
Example usage
Typical usage of the YAML library:
Should produce the following output:
Typical usage of the VTK libray: