FloatingArrayDesign / MoorDyn

a lumped-mass mooring line model intended for coupling with floating structure codes
BSD 3-Clause "New" or "Revised" License
66 stars 37 forks source link

Time integrators #182

Closed sanguinariojoe closed 3 weeks ago

sanguinariojoe commented 6 months ago

THIS PR DEPENDS ON https://github.com/FloatingArrayDesign/MoorDyn/pull/224

Well, I am afraid this will require quite a bit of discussion on several of the changes I implemented:

sanguinariojoe commented 5 months ago

Hey guys! I uploaded a Newmark scheme. An Average Constant Acceleration (ACA) one to be more specific, with relaxation, which is able to remain stable with significantly larger time steps.

@mattEhall and @RyanDavies19 , it would be great if you can review this and give feedback.

RyanDavies19 commented 3 months ago

@sanguinariojoe just keeping you in the loop, Matt and I have both been pretty busy with other work so we haven't gotten the chance to dig too much into this yet. It's still on our radar, and hopefully I'll be able to get to it soon. Currently I am working on adding vortex induced vibrations to MoorDyn.

sanguinariojoe commented 3 months ago

Currently I am working on adding vortex induced vibrations to MoorDyn

Cool! I was told to avoid this on the past because apparently those are engineered out, so our digital twins shall not take them into account

sanguinariojoe commented 1 month ago

Hey @RyanDavies19 !

This branch is evolving quite much... I am indeed never using dev branch anymore.

I really think we should start addressing this before both branches start diverging too much

RyanDavies19 commented 1 month ago

@sanguinariojoe agreed. I will be reviewing today now that I have some time available for MDC work. Thank you for including the docs, that is very helpful. My biggest concerns are making sure that 1) we retain backwards compatibility in the form of input files, and 2) the new input file is well documented (which it seems you've already done a lot of). Many of the questions I see in the OpenFAST issues/forum are from confusions between the two different code versions so keeping things clear will be important.

sanguinariojoe commented 1 month ago

Well, actually i want to document it a bit further. The discussion on the relaxation factor is unfinished

On Thu, 13 Jun 2024, 18:31 RyanDavies19, @.***> wrote:

@sanguinariojoe https://github.com/sanguinariojoe agreed. I will be reviewing today now that I have some time available for MDC work. Thank you for including the docs, that is very helpful. My biggest concerns are making sure that 1) we retain backwards compatibility in the form of input files, and 2) the new input file is well documented (which it seems you've already done a lot of). Many of the questions I see in the OpenFAST issues/forum are from confusions between the two different code versions so keeping things clear will be important.

— Reply to this email directly, view it on GitHub https://github.com/FloatingArrayDesign/MoorDyn/pull/182#issuecomment-2166172012, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMXKKGTIY4VDLNX2Z7AO23ZHHCPRAVCNFSM6AAAAABB2HBDPWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNRWGE3TEMBRGI . You are receiving this because you were mentioned.Message ID: @.***>

RyanDavies19 commented 1 month ago

@sanguinariojoe okay that would be good. I'll add comments in the code where some slight changes might be needed. Once you get those docs done it would be good to read

sanguinariojoe commented 3 weeks ago

I rebased this PR on https://github.com/FloatingArrayDesign/MoorDyn/pull/224, so that PR shall be merged first

RyanDavies19 commented 3 weeks ago

@sanguinariojoe there should be a PR up to OpenFAST/dev tomorrow afternoon with updated reg tests for comparison using #224

sanguinariojoe commented 3 weeks ago

Nice! I think #224 can be merged. Anyway the verifications against openfast dev have to be made manually

On Mon, 24 Jun 2024, 18:27 RyanDavies19, @.***> wrote:

@sanguinariojoe https://github.com/sanguinariojoe there should be a PR up to OpenFAST/dev tomorrow afternoon with updated reg tests for comparison using #224 https://github.com/FloatingArrayDesign/MoorDyn/pull/224

— Reply to this email directly, view it on GitHub https://github.com/FloatingArrayDesign/MoorDyn/pull/182#issuecomment-2186963554, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMXKKDK4LUHYEOQB4AIWO3ZJBCHXAVCNFSM6AAAAABB2HBDPWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOBWHE3DGNJVGQ . You are receiving this because you were mentioned.Message ID: @.***>

RyanDavies19 commented 3 weeks ago

@sanguinariojoe I agree it looks good to go. OpenFAST dev is a few weeks away from merging into main so we should be able to get the new reg tests in by then. If not it's a simple fix to point to the dev branch. Only remaining question is the GH artifact comment I made on #224. It would be good to have comparison plots if the test fails

RyanDavies19 commented 3 weeks ago

@sanguinariojoe Added a few more docs reviews. I will open a PR to your branch with the fixes. Overall though they look really good and are very helpful. Thanks for putting them together!

RyanDavies19 commented 3 weeks ago

See https://github.com/OpenFAST/r-test/issues/127 for updated reg tests

sanguinariojoe commented 3 weeks ago

See OpenFAST/r-test#127 for updated reg tests

Hey @RyanDavies19 , the new tests do not work to me... In OpenFAST I mean. It seems that all the drivers named like BodiesAndRods_driver.dvr shall be replaced by md_driver.inp

After making all those replacements, it complains about the lack of the reference results. For instance:

131: Error: Error: file does not exist at "/home/pepe/CoreMarine/openfast.action/openfast/reg_tests/../reg_tests/r-test/modules/moordyn/md_BodiesAndRods/driver.MD.out"

Finally, several tests are not printing any output to compare with, so the driver.MD.out will be empty.

I suggest merging this and we can carry out the verifications when they are ready. dev branch is precisely meant for that

RyanDavies19 commented 3 weeks ago

@sanguinariojoe Agreed lets merge. I talked with Matt and that's fine. Apologies for confusion with https://github.com/OpenFAST/r-test/pull/127, I should have opened that as a draft. It is a work in progress.

sanguinariojoe commented 3 weeks ago

Hurray! Thanks guys!