CliMA / Oceananigans.jl

🌊 Julia software for fast, friendly, flexible, ocean-flavored fluid dynamics on CPUs and GPUs
https://clima.github.io/OceananigansDocumentation/stable
MIT License
991 stars 194 forks source link

`WENO5` is very different from other advection schemes #2098

Closed francispoulin closed 2 years ago

francispoulin commented 2 years ago

To get the string name of an advection scheme I can do something like the following:

scheme1 = UpwindBiasedThirdOrder()
string(scheme1)[1:end-2]

which produces "UpwindBiasedThirdOrder". I am doing this to get titles in figures to compare the results of the different schemes.

However, when I try doing something similar with WENO5, it fails because it is designed very differenetly:

scheme2 = WENO5()
string(scheme2)

which instead produces

"WENO5 advection scheme with: \n ├── X regular \n ├── Y regular \n └── Z regular"

I am not suggesting anything is wrong but can someone suggest how I might be able to get string names from these advection schemes more easily?

@simone-silvestri ?

glwagner commented 2 years ago

@simone-silvestri

simone-silvestri commented 2 years ago

Makes sense, I'll change the print function to make it consistent with other advection schemes

2099

francispoulin commented 2 years ago

Thank you @simone-silvestri !

navidcy commented 2 years ago

Let's close the issue when #2099 is merged. :)

francispoulin commented 2 years ago

Yes, very good idea. Thanks for doing just that.

Francis


From: Navid C. Constantinou @.> Sent: Sunday, December 5, 2021 8:58:49 PM To: CliMA/Oceananigans.jl @.> Cc: Francis Poulin @.>; State change @.> Subject: Re: [CliMA/Oceananigans.jl] WENO5 is very different from other advection schemes (Issue #2098)

Let's close the issue when #2099https://github.com/CliMA/Oceananigans.jl/pull/2099 is merged. :)

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHubhttps://github.com/CliMA/Oceananigans.jl/issues/2098#issuecomment-986369683, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AB63PQLDYFOGAU3RT26VFPDUPQKFTANCNFSM5JNBEH2Q. Triage notifications on the go with GitHub Mobile for iOShttps://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Androidhttps://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

navidcy commented 2 years ago

@francispoulin is this a better solution?

julia> scheme1 = UpwindBiasedThirdOrder()
UpwindBiasedThirdOrder()

julia> scheme2 = WENO5()
┌ Warning: defaulting to uniform WENO scheme with Float64 precision, use WENO5(grid = grid) if this was not intended
└ @ Oceananigans.Advection ~/Research/Oceananigans.jl/src/Advection/weno_fifth_order.jl:145
WENO5 advection scheme with:
    ├── X regular
    ├── Y regular
    └── Z regular

julia> string(typeof(scheme1).name.wrapper)
"UpwindBiasedThirdOrder"

julia> string(typeof(scheme2).name.wrapper)
"WENO5"
navidcy commented 2 years ago

perhaps we rename the issue to something along the lines of

show() method for WENO5 is very different from other advection schemes`

?

francispoulin commented 2 years ago

Closed after the disucssion in #2099