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

Feedback #2

Closed
ranjanan opened this issue Oct 29, 2018 · 1 comment
Closed

Feedback #2

ranjanan opened this issue Oct 29, 2018 · 1 comment
Assignees
Labels
feature 🌟 Something new and shiny performance 🏍️ So we can get the wrong answer even faster

Comments

@ranjanan
Copy link

  • Here const all the things.
  • For embarrasingly parallel : pmap (others = @parallel, DistributedArrays, MPIArrays)
  • GPU: CuArrays (quick and dirty), GPUArrays (architecture agnostic)
@ali-ramadhan ali-ramadhan added feature 🌟 Something new and shiny performance 🏍️ So we can get the wrong answer even faster labels Feb 9, 2019
@ali-ramadhan
Copy link
Member

Thanks for the feedback @ranjanan !

  • Instead of using const we went with structs.
  • CuArrays have worked quite well but still working on writing performant CPU/GPU shared kernels (hoping GPUifyLoops.jl will help here).
  • Multicore parallelism is a bit lower priority right now but have had some pretty good results with DistributedArrays although I think MPIArrays will be the way forward for us here.

@ali-ramadhan ali-ramadhan self-assigned this Mar 17, 2019
hennyg888 added a commit that referenced this issue Jun 26, 2021
navidcy added a commit that referenced this issue Jan 16, 2022
Split tests into smaller bits and pieces (take #2)
tomchor added a commit that referenced this issue Mar 23, 2023
…#2) (#2990)

* ZDirection -> NegativeZDirection (doctests pass locally atm)

* @info -> @warn

* comment parts of the documentation

* uncomment tilted bbl example (docs still build locally w this change)

* bring back other doc pages (still builds)

* full docs

* adds review suggestions

* bugfix

* better unit-vector validation + fix coriolis tests

---------

Co-authored-by: Navid C. Constantinou <navidcy@users.noreply.github.com>
simone-silvestri added a commit that referenced this issue Apr 11, 2023
* remove tullio dependency

* bugfix

* add abs to abstract operations

* solved the issue

* probably not super efficient

* indices for gridmetricoperations

* just remove tullio

* change timescale definition

* do not introduce indices now

* Update src/Advection/cell_advection_timescale.jl

Co-authored-by: Gregory L. Wagner <wagner.greg@gmail.com>

* Update src/Diagnostics/cfl.jl

* accurate_cell_advection_timescale -> cell_advection_timescale

---------

Co-authored-by: Gregory L. Wagner <wagner.greg@gmail.com>
Co-authored-by: Navid C. Constantinou <navidcy@users.noreply.github.com>
simone-silvestri added a commit that referenced this issue Sep 19, 2023
…cFreeSurfaceModel` and `NonhydrostaticModel` (#3125)

* comment

* fixed tag problems

* bugfix

* Update scalar_biharmonic_diffusivity.jl

* Update src/Distributed/multi_architectures.jl

Co-authored-by: Navid C. Constantinou <navidcy@users.noreply.github.com>

* Update src/Distributed/partition_assemble.jl

Co-authored-by: Navid C. Constantinou <navidcy@users.noreply.github.com>

* Update src/ImmersedBoundaries/ImmersedBoundaries.jl

Co-authored-by: Navid C. Constantinou <navidcy@users.noreply.github.com>

* Update src/ImmersedBoundaries/active_cells_map.jl

Co-authored-by: Navid C. Constantinou <navidcy@users.noreply.github.com>

* Update src/Distributed/interleave_comm_and_comp.jl

Co-authored-by: Navid C. Constantinou <navidcy@users.noreply.github.com>

* Clean up batched tridiagonal solver and vertically implicit solver

* Fix bug in batched tridiagonal solver

* bugfix

* Try to fix multi region immersed boundary issue

* Hopefully fix immersed boundary grid constructor

* Another fix

* fixed project and manifest

* convert instead of FT

* export KernelParameters

* remove FT

* removed useless where FT

* small bugfix

* update manifest

* remove unbuffered communication

* little bit of a cleanup

* removed `views` comment

* couple of bugfixes

* fixed tests

* probably done

* same thing for nonhydrostatic model

* include file

* bugfix

* prepare for nonhydrostatic multiregion

* also here

* bugfix

* other bugfix

* fix closures

* bugfix

* simplify

* 2D leith requires 2 halos!

* AMD and Smag require 1 halo!

* wrong order

* correct halo handling for diffusivities

* correct Leith formulation + fixes

* `only_local_halos` kwarg in `fill_halo_regions!`

* bugfix

* FT on GPU

* bugfix

* bugfix

* last bugfix?

* removed all offsets from kernels + fixed all tests

* fix `_compute!`

* finished

* fixed broken tests

* fixed docs

* miscellaneous changes

* bugfix

* removed tests for vertical subdivision

* test corner passing

* correction

* retry

* fixed all problems

* Added a validation example

* fixed tests

* try new test

* fill send buffers in the correct place

* fixed comments

* define async

* pass the grid

* bugfix

* fix show method

* RefValue for mpi_tag

* comment

* add catke preprint

* remove warning; add ref to catke preprint

* some code cleanup

* correct the example

* Update src/TurbulenceClosures/vertically_implicit_diffusion_solver.jl

Co-authored-by: Navid C. Constantinou <navidcy@users.noreply.github.com>

* bugfix

* Refactor unit tests

* Generalize regridding for lat-lon

* bugfix

* Add newline

* small correction

* new tests

* bugfix

* bugfix

* back for testing

* update manifest

* more options

* more

* finished

* test hypothesis

* fixed bug - correct speed now

* add space

* bugfix

* test

* more info

* removed left-right connected computation

* bugfix

* remove info

* improve

* typo

* bugfix

* bugfix

* correct comments

* bugfix

* bugfix prescribed velocities

* fixes

* ok on mac

* bugfix

* bug fixed

* bugfixxed

* new default

* bugfix

* remove <<<<HEAD

* bugfix PrescribedVelocityFields

* default in another PR

* bugfix

* flat grids only in Grids

* last bugfix

* bugfix

* try partial cells

* bugfix

* bugfix

* Update test_turbulence_closures.jl

* small fixes

* rework IBG and MRG

* Update src/TurbulenceClosures/vertically_implicit_diffusion_solver.jl

* small bugfix

* remove multiregion ibg with arrays for the moment

* bugfix

* little cleaner

* fixed tests

* see what the error is

* allow changing halos from checkpointer

* test it

* finally fixed it

* better naming

* bugfix

* bugfix

* bugfix

* bugfix

* removed useless tendency

* small fix

* dummy commit

* fix active cell map

* comment

* bugfix

* bugfix

* removed useless tendency

* maybe just keep it does not harm too much

* should have fixed it?

* let's go now

* done

* bugfix

* no need for this

* convert Δt in time stepping

* maximum

* minimum substeps

* more flexibility

* bugfix

* mutlidimensional

* fallback methods

* test a thing

* change

* chnage

* change

* change

* update

* update

* new offsets + return to previous KA

* bugfix

* bugfixxed

* remove debugging

* going back

* more robus partitioning

* quite new

* bugfix

* updated Manifest

* build with 1.9.3

* switch boundary_buffer to required_halo_size

* bugfix

* Update src/Models/HydrostaticFreeSurfaceModels/single_column_model_mode.jl

Co-authored-by: Gregory L. Wagner <wagner.greg@gmail.com>

* Update src/Models/HydrostaticFreeSurfaceModels/update_hydrostatic_free_surface_model_state.jl

Co-authored-by: Gregory L. Wagner <wagner.greg@gmail.com>

* bugfix

* biharmonic requires 2 halos

* buggfix

* compute_auxiliaries!

* bugfix

* fixed it

* little change

* some changes

* bugfix

* bugfix

* bugfixxed

* another bugfix

* compute_diffusivities!

* required halo size

* all fixed

* shorten line

* fix comment

* remove abbreviation

* remove unused functions

* better explanation of the MPI tag

* Update src/ImmersedBoundaries/active_cells_map.jl

Co-authored-by: Navid C. Constantinou <navidcy@users.noreply.github.com>

* Update src/Solvers/batched_tridiagonal_solver.jl

Co-authored-by: Navid C. Constantinou <navidcy@users.noreply.github.com>

* change name

* docstring

* name change on rank

* interior active cells

* calculate -> compute

* fixed tests

* do not compute momentum in prescribed velocities

* DistributedComputations

* DistributedComputations part #2

* bugfix

* fixed the docs

---------

Co-authored-by: Navid C. Constantinou <navidcy@users.noreply.github.com>
Co-authored-by: Gregory L. Wagner <wagner.greg@gmail.com>
navidcy added a commit that referenced this issue Sep 20, 2023
…cFreeSurfaceModel` and `NonhydrostaticModel` (#3125)

* comment

* fixed tag problems

* bugfix

* Update scalar_biharmonic_diffusivity.jl

* Update src/Distributed/multi_architectures.jl

Co-authored-by: Navid C. Constantinou <navidcy@users.noreply.github.com>

* Update src/Distributed/partition_assemble.jl

Co-authored-by: Navid C. Constantinou <navidcy@users.noreply.github.com>

* Update src/ImmersedBoundaries/ImmersedBoundaries.jl

Co-authored-by: Navid C. Constantinou <navidcy@users.noreply.github.com>

* Update src/ImmersedBoundaries/active_cells_map.jl

Co-authored-by: Navid C. Constantinou <navidcy@users.noreply.github.com>

* Update src/Distributed/interleave_comm_and_comp.jl

Co-authored-by: Navid C. Constantinou <navidcy@users.noreply.github.com>

* Clean up batched tridiagonal solver and vertically implicit solver

* Fix bug in batched tridiagonal solver

* bugfix

* Try to fix multi region immersed boundary issue

* Hopefully fix immersed boundary grid constructor

* Another fix

* fixed project and manifest

* convert instead of FT

* export KernelParameters

* remove FT

* removed useless where FT

* small bugfix

* update manifest

* remove unbuffered communication

* little bit of a cleanup

* removed `views` comment

* couple of bugfixes

* fixed tests

* probably done

* same thing for nonhydrostatic model

* include file

* bugfix

* prepare for nonhydrostatic multiregion

* also here

* bugfix

* other bugfix

* fix closures

* bugfix

* simplify

* 2D leith requires 2 halos!

* AMD and Smag require 1 halo!

* wrong order

* correct halo handling for diffusivities

* correct Leith formulation + fixes

* `only_local_halos` kwarg in `fill_halo_regions!`

* bugfix

* FT on GPU

* bugfix

* bugfix

* last bugfix?

* removed all offsets from kernels + fixed all tests

* fix `_compute!`

* finished

* fixed broken tests

* fixed docs

* miscellaneous changes

* bugfix

* removed tests for vertical subdivision

* test corner passing

* correction

* retry

* fixed all problems

* Added a validation example

* fixed tests

* try new test

* fill send buffers in the correct place

* fixed comments

* define async

* pass the grid

* bugfix

* fix show method

* RefValue for mpi_tag

* comment

* add catke preprint

* remove warning; add ref to catke preprint

* some code cleanup

* correct the example

* Update src/TurbulenceClosures/vertically_implicit_diffusion_solver.jl

Co-authored-by: Navid C. Constantinou <navidcy@users.noreply.github.com>

* bugfix

* Refactor unit tests

* Generalize regridding for lat-lon

* bugfix

* Add newline

* small correction

* new tests

* bugfix

* bugfix

* back for testing

* update manifest

* more options

* more

* finished

* test hypothesis

* fixed bug - correct speed now

* add space

* bugfix

* test

* more info

* removed left-right connected computation

* bugfix

* remove info

* improve

* typo

* bugfix

* bugfix

* correct comments

* bugfix

* bugfix prescribed velocities

* fixes

* ok on mac

* bugfix

* bug fixed

* bugfixxed

* new default

* bugfix

* remove <<<<HEAD

* bugfix PrescribedVelocityFields

* default in another PR

* bugfix

* flat grids only in Grids

* last bugfix

* bugfix

* try partial cells

* bugfix

* bugfix

* Update test_turbulence_closures.jl

* small fixes

* rework IBG and MRG

* Update src/TurbulenceClosures/vertically_implicit_diffusion_solver.jl

* small bugfix

* remove multiregion ibg with arrays for the moment

* bugfix

* little cleaner

* fixed tests

* see what the error is

* allow changing halos from checkpointer

* test it

* finally fixed it

* better naming

* bugfix

* bugfix

* bugfix

* bugfix

* removed useless tendency

* small fix

* dummy commit

* fix active cell map

* comment

* bugfix

* bugfix

* removed useless tendency

* maybe just keep it does not harm too much

* should have fixed it?

* let's go now

* done

* bugfix

* no need for this

* convert Δt in time stepping

* maximum

* minimum substeps

* more flexibility

* bugfix

* mutlidimensional

* fallback methods

* test a thing

* change

* chnage

* change

* change

* update

* update

* new offsets + return to previous KA

* bugfix

* bugfixxed

* remove debugging

* going back

* more robus partitioning

* quite new

* bugfix

* updated Manifest

* build with 1.9.3

* switch boundary_buffer to required_halo_size

* bugfix

* Update src/Models/HydrostaticFreeSurfaceModels/single_column_model_mode.jl

Co-authored-by: Gregory L. Wagner <wagner.greg@gmail.com>

* Update src/Models/HydrostaticFreeSurfaceModels/update_hydrostatic_free_surface_model_state.jl

Co-authored-by: Gregory L. Wagner <wagner.greg@gmail.com>

* bugfix

* biharmonic requires 2 halos

* buggfix

* compute_auxiliaries!

* bugfix

* fixed it

* little change

* some changes

* bugfix

* bugfix

* bugfixxed

* another bugfix

* compute_diffusivities!

* required halo size

* all fixed

* shorten line

* fix comment

* remove abbreviation

* remove unused functions

* better explanation of the MPI tag

* Update src/ImmersedBoundaries/active_cells_map.jl

Co-authored-by: Navid C. Constantinou <navidcy@users.noreply.github.com>

* Update src/Solvers/batched_tridiagonal_solver.jl

Co-authored-by: Navid C. Constantinou <navidcy@users.noreply.github.com>

* change name

* docstring

* name change on rank

* interior active cells

* calculate -> compute

* fixed tests

* do not compute momentum in prescribed velocities

* DistributedComputations

* DistributedComputations part #2

* bugfix

* fixed the docs

---------

Co-authored-by: Navid C. Constantinou <navidcy@users.noreply.github.com>
Co-authored-by: Gregory L. Wagner <wagner.greg@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature 🌟 Something new and shiny performance 🏍️ So we can get the wrong answer even faster
Projects
None yet
Development

No branches or pull requests

2 participants