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

Wind #50

Merged
merged 2 commits into from
Jul 14, 2023
Merged

Wind #50

merged 2 commits into from
Jul 14, 2023

Conversation

hinata235
Copy link
Contributor

MMG model is changed to take wind force into account.

data.jl

  • Added functions for ship superstructure data get_structure_params

mmg.jl

ShipMMG.jl

  • Changed due to changes in data.jl,mmg.jl

test_mmg.jl

  • Changed due to the changes in data.jl,mmg.jl,ShipMMG.jl

Other changes

  • Change Psi from uppercase to lowercase (Ψ→ψ)

Addition of wind force term.
Addition of wind force terms.
@taiga4112 taiga4112 self-requested a review July 12, 2023 05:55
@taiga4112 taiga4112 self-assigned this Jul 12, 2023
Copy link
Contributor

@taiga4112 taiga4112 left a comment

Choose a reason for hiding this comment

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

Good code!! Thank you for your contributes.

In the viewpoint of minimum required functional modeling of MMG simulation, please modify your code as my review.

  • Use Mmg3DofWindForceMomentParams instead of Mmg3DofStructureParams and organize where params are placed.

Comment on lines +472 to +487
# Arguments
- `A_OD::T`
- `A_F::T`
- `A_L::T`
- `H_BR::T`
- `H_C::T`
- `C::T`
"""
@with_kw mutable struct Mmg3DofStructureParams{T}
A_OD::T
A_F::T
A_L::T
H_BR::T
H_C::T
C::T
end
Copy link
Contributor

Choose a reason for hiding this comment

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

Various parameters need to be divided into the parameters needed for the simulation and the information to define them.

From your PR, I think that the following struct should be defined as essential information for MMG simulation.

@with_kw mutable struct Mmg3DofWindForceMomentParams{T}
    A_F::T
    A_L::T
    C_X_wind::T
    C_Y_wind::T
    C_N_wind::T
end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the comments!
It seems difficult to change Mmg3DofStructureParamstoMmg3DofWindForceMomentParams, because C_X_wind,C_Y_wind,C_N_wind are calculated sequentially using Mmg3DofStructureParams in mag_3dof_model!.
We need to have a discussion about this.

@taiga4112 taiga4112 changed the base branch from dev to feature/wind_force July 12, 2023 13:42
@taiga4112
Copy link
Contributor

OK. I will show the example implementation of using Mmg3DofWindForceMomentParams in this branch.

@taiga4112 taiga4112 merged commit b8027fe into ShipMMG:feature/wind_force Jul 14, 2023
@taiga4112
Copy link
Contributor

@hinata235
Before my updating, please check and update the test code.
If the test code runs, some error occurred.

Following command shows the example for running test code.

julia > ] # package mode
(@v1.9) pkg> activate .
(ShipMMG) test 

@hinata235
Copy link
Contributor Author

@taiga4112
I ran the above test code and no error occurred in my case.
Here is my operating environment.
It may be due to the difference in Julia's ver.

Julia> versioninfo()
Julia Version 1.8.2
Commit 36034abf260 (2022-09-29 15:21 UTC)
Platform Info:
OS: macOS (arm64-apple-darwin21.3.0)
CPU: 10 × Apple M1 Max
WORD_SIZE: 64
LIBM: libopenlibm
LLVM: libLLVM-13.0.1 (ORCJIT, apple-m1)
Threads: 1 on 8 virtual cores

@taiga4112
Copy link
Contributor

I am not interested in your PC's execution environment.

Anyhow, I cannot find the Mmg3DofStructureParams params in the test code from your GitHub PR branch. Because of this reason, I can get the following error.

mmg.jl KVLCC2_L7 turning: Error During Test at /home/mitsuyuki/Dropbox/workspace/dev-ShipMMG/ShipMMG.jl/test/test_mmg.jl:3
  Got exception outside of a @test
  MethodError: no method matching mmg_3dof_simulate(::Mmg3DofBasicParams{Float64}, ::Mmg3DofManeuveringParams{Float64}, ::StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}, ::Vector{Float64}, ::Vector{Float64}, ::Vector{Float64}, ::Float64; u0::Float64, v0::Float64, r0::Float64)
  
  Closest candidates are:
    mmg_3dof_simulate(::Mmg3DofBasicParams, ::Mmg3DofManeuveringParams, ::Mmg3DofStructureParams, ::Any, ::Any, ::Any, ::Any, ::Any; u0, v0, r0, x0, y0, ψ0, ρ, algorithm, reltol, abstol)
     @ ShipMMG ~/Dropbox/workspace/dev-ShipMMG/ShipMMG.jl/src/mmg.jl:543
  
  Stacktrace:
    [1] macro expansion
      @ ~/Dropbox/workspace/dev-ShipMMG/ShipMMG.jl/test/test_mmg.jl:16 [inlined]
    [2] macro expansion
      @ /opt/julias/julia-1.9.2/share/julia/stdlib/v1.9/Test/src/Test.jl:1498 [inlined]
    [3] top-level scope
      @ ~/Dropbox/workspace/dev-ShipMMG/ShipMMG.jl/test/test_mmg.jl:5
    [4] include(fname::String)
      @ Base.MainInclude ./client.jl:478
    [5] macro expansion
      @ ~/Dropbox/workspace/dev-ShipMMG/ShipMMG.jl/test/runtests.jl:9 [inlined]
    [6] macro expansion
      @ /opt/julias/julia-1.9.2/share/julia/stdlib/v1.9/Test/src/Test.jl:1498 [inlined]
    [7] top-level scope
      @ ~/Dropbox/workspace/dev-ShipMMG/ShipMMG.jl/test/runtests.jl:7
    [8] include(fname::String)
      @ Base.MainInclude ./client.jl:478
    [9] top-level scope
      @ none:6
   [10] eval
      @ ./boot.jl:370 [inlined]
   [11] exec_options(opts::Base.JLOptions)
      @ Base ./client.jl:280
   [12] _start()
      @ Base ./client.jl:522

Why can your test code run without error? It's quite strange for me...

@hinata235
Copy link
Contributor Author

@taiga4112
It's my mistake. I had the wrong branch. I ran the test again on the correct branch and confirmed the same error.
I will modify test_mmg.jl and commit again.

@taiga4112
Copy link
Contributor

#54

@taiga4112
Copy link
Contributor

taiga4112 commented Jul 15, 2023

@hinata235
Still there are many errors after running your test code.
Please send PR before you check your code by running test code.

Following command is to run the test code of this repository.
#50 (comment)

@taiga4112
Copy link
Contributor

@hinata235
This page shows the result of running your test code on GitHub server.

Please check it.

@taiga4112
Copy link
Contributor

@hinata235
This PR is one of the implementation of Mmg3DofWindForceMomentParams instead of Mmg3DofStructureParams. Please check it.

The point of this PR is to separate between the definition of core parameters for calculating wind force & moment and the estimation method of this core parameters.

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.

Consider the impact of wind, waves, and currents on the maneuvering model
2 participants