OpenFAST / openfast

Main repository for the NREL-supported OpenFAST whole-turbine and FAST.Farm wind farm simulation codes.
http://openfast.readthedocs.io
Apache License 2.0
691 stars 458 forks source link

SubDyn Direction Cosines #1404

Closed Sam-Ramsahoye closed 1 year ago

Sam-Ramsahoye commented 1 year ago

Hi,

Currently, the user can specify direction cosines for SubDyn elements in the SubDyn primary input file but they aren't used. Are there any plans to feed them through into the analysis?

Would it simply be a case of replacing the GetDirCos in SD_FEM.f90 (modules/subdyn/src/SD_FEM.f90) with a lookup of Init%COSMs(I,:) and an appropriate reshape?

Best wishes, Sam

ebranlard commented 1 year ago

Hi,

Out of curiosity, why would you like to introduce direction cosines matrices table lookup? For non circular elements, this could be useful, but if in the future we add non-circular elements, we might introduce a way to easily define the orientation of these elements without having to define the full direction cosine (the z axis being still defined along the member, then it's a matter of defining a rotation around it somehow). I haven't fully thought it through though. I'll let @jjonkman comment if he thinks there is value in going the Cosine matrix lookup way.

If we want to introduce this table lookup of COSM, the changes are fairly simple, but there are a couple of things to look for. Here's the changes I can see:

If we decide to go this way, and you have a bit of time to try it out, I'll be happy to progressively give you feedback along the way on a pull request.

Sam-Ramsahoye commented 1 year ago

@ebranlard Thanks for reaching out!

I'm pretty sure Init%COSMs is defined when the SubDyn inputs are read - I'm not sure I'd have to add anything in there.

I hadn't considered the renumbering of the elements but agree with your solution (to store a COSMID mapping in the FEM element).

There are two reasons why the user-specified member cosines are useful to us:

  1. remove the need to transform loads in post-processing (not a great reason)
  2. as you say, for non-circular members and their orientation

As to exactly how they are specified (cosines matrices or angle), I can see how defining an angle might make things appear simpler but ultimately the user must know the reference to which it's defined and that complicates things slightly. user-specified cosine matrices have their own problems: the user may accidentally define a set of non-orthonormal vectors.

I'm happy to try and get something running and submit a PR: I don't have much experience with Fortran specifically but I'm quite familiar with compiling OpenFAST and it's individual modules too. If I come up with something that works, I'll give you a shout.

Thanks, Sam

ebranlard commented 1 year ago

Hi @Sam-Ramsahoye, regarding the reading, COSMs is indeed read at init, but I was concerned about the column COSMID in the Member table, which currently is not present in any input file.

Otherwise, let's see what jason says, if it makes sense to implement it, it would be great to have your help! Taking a stab at one of those fairly focused task can be a great way to learn about fortran!

Cheers

jjonkman commented 1 year ago

I would just add that, in an upcoming project, we are planning to add support for members with rectangular cross sections in SubDyn and HydroDyn. (Not arbitrary cross sections, but rectangular in addition to the cylindrical sections supported now.) We have not worked out the details yet, but we're thinking of avoiding the use of full direction cosine matrices because simpler user-input is possible if, e.g., one of the axes of the cross section can always be assumed to be horizontal, which is likely the case for most any reasonable substructure.

Best regards,

Sam-Ramsahoye commented 1 year ago

@jjonkman @ebranlard Thanks both.

I agree on balance the use of an angle is simpler and it would allow you to remove the cosine matrices section entirely as it can be appended as a column in the member definition.

Based on the above and looking at the source and documentation, am I right in thinking user-defined non-circular cross-section properties aren't used?

We've got quite a funky jacket transition piece parameterize that has a number of different sections types (channel, rectangular, rectangular hollow shell, I section) and and so this functionality would be nice to have. I might have a go at adding this in too.

Thanks

ebranlard commented 1 year ago

Hi @Sam-Ramsahoye,

It sounds like your implementation needs are inline with some of what we plan to do. Since you might need this sooner that we are ready to start, we can try to assist you in your implementation.

It might take us some time to agree on a definition for the angle, so I think it makes sense to go ahead with implementing the table lookup of direction cosine matrices. Hopefully my previous message will help you start things.

When you get to the implementation of other element types, feel free to ask questions. I'm giving a couple of thoughts below.

Keep us in touch, I'd be happy to progressively guide you in the implementation.

Sam-Ramsahoye commented 1 year ago

@ebranlard Thanks for your message - I'm really keen to contribute something to the code base as we've been using OpenFAST just as users for a while now.

I'm actually going to be on leave for the next two weeks but I'm keen to do what I can ASAP.

FYI this kind of thing is hard to get funding for through my employer, so this is more of a self-directed learning exercise I intend to do in my own time. In this light, I'm not waiting for anyone at NREL to do it ahead of me as it should be a simple first exercise for me to get started with: issues I have will largely be with the Fortran syntax more than anything else, although I'm not entirely unfamiliar with it and at least in my experience, it's quite a simple language.

We only need "arbitrary" cross-sections in SubDyn, rather than HydroDyn too. To this end, I'm actually keenest on doing something that's inline with what has been started in the SubDyn input files already - specifying A, Ax, Ay, Ixx, Iyy and J. This, with orientation specification, is the most flexible possible solution.

I'll keep in touch :)

Cheers, Sam

samuel-ramsahoye commented 1 year ago

@ebranlard I'm starting to put together a PR for you to review - I'll try and have a draft ready by the end of this week.

I think I've got most of the base functionality in there now but it could do with tidying.

This includes user-specified cosines and the handling of arbitrary cross-sections. Something that I'm missing at the moment is handling the case when the user doesn't specify the COSMID member column or if the user specifies a -1 in this column. I'll add before I make the PR so that SubDyn input files can remain the same.

I've added a new member id for members with arbitrary section properties, "idMemberSpecial", in a similar light to idMemberBeam, idMemberCable etc. I'm going to rename special to arbitrary so it's inline with the input file definition. Members with arbitrary cross-section properties currently aren't refined, but this is an easy thing for me to add too.

I have made a small change to ElemPropType: it now stores kappa_x and kappa_y rather just kappa. I can undo if necessary although this does allow us to create stiffness matrices in much the same way as before. Moreover, other similar properties like Ixx and Iyy weren't assumed to be the same in both x and y before either so this felt natural.

How do you normally go about testing? In the past when I've written some small FE codes, I've checked the assembled mass and stiffness matrices for a range of cases.

Cheers, Sam

ebranlard commented 1 year ago

Hi @samuel-ramsahoye

Thanks for all the work, this seems to make sense. idMemberArbitrary makes sense.

To be backward compatible for a little while and nice to the user, you can read the member table string by string and then use ReadIAryFromStr to detect the number of columns that are valid and numeric. If the number of numeric column is 6 on the first line, warn the user that they are using an old format and should have -1 on the last columns. It will make the code a bit ugly at first, but we can remove this in a future release.

One of the reason I'm suggesting this backward compatibility, is that we can run our r-test with your new version of the code (provided we comment the reading of your new Arbitrary tables at first, or, you also makes this section optional...), without having to change the input files of the r-test. If the r-test pass, that gives us some confidence that your changes do not introduce major issues in existing tests.

I think that it would be a good first step for the pull request.

Then, once we can see that the pull request runs, we can work on updating the r-test. We can uncomment the part of the code that reads the new arbitrary tables. The r-test will now fail. You'll have to create a branch of r-test, edit all the SubDyn input files (adding the -1 in the COSID column, adding your arbitrary table), add one or two very short r-test for the subdyn driver to test your new functionalities, and then point your openfast submodule r-test to your branch of r-test. It's a bit of a tricky process if you are not familiar with it, which is why I recommend doing a pull request that is compatible with old input files at first.

The SubDyn driver tests (see examples here) that you add to the r-test repository to test your new feature, should be "canonical tests", that run quickly, creates a very small set of meaningful outputs, preferably comparable with an analytical solution or a known solution, and documented with a README file. (I'm not saying the current tests all comply to that..). Before creating such test, we can discuss a bit what test could make sense.

We would probably also like to discuss the format of the table of arbitrary properties with @jjonkman. Do you have an example of your input file table that you could share here?

Thanks again for your work!

samuel-ramsahoye commented 1 year ago

@ebranlard Thanks for your guidance above!

I've put together a draft PR now, perhaps you could take a look at the proposed changes to the code? Please be critical as I'd like to know what about it could be improved, some of it's definitely hacky and not particularly neat.

As I said in the PR, I've really just made sure it compiles and runs rather than checking the physical correctness just yet. I'll pull together some canonical tests soon.

Cheers, Sam

ebranlard commented 1 year ago

Thanks a lot, I'll try to review before the end of the week. I wonder why the compilation in debug fails. It seems to be something related to the Type file, it's kind of weird.

ebranlard commented 1 year ago

Addressed in
https://github.com/OpenFAST/openfast/pull/1413