type piracy in getindex method #277
Labels
bug
Something isn't working
Comments
|
@charleskawczynski would it be possible to schedule some time to discuss how to remove these additional methods? |
bors bot
added a commit
that referenced
this issue
Aug 31, 2023
310: Domain stretching r=kmdeck a=kmdeck ## Purpose Allows for variable domain stretching (finer resolution at the top of the domain compared to the bottom). Whenever we need dz in source code, we obtain it using the function `get_Δz`, which uses ClimaCore to extract the `dz` values. so we do not make the assumption of uniform spacing in src code. (we do in some tests, but that seems OK if those tests use uniform spacing.) ## To-do Future PRs: fix piracy and add the aqua test back in: #277 #311 ## Content 1. Adds domain stretching to all domains with a z coordinate. 2. Renames height to depth for the SphericalShell domain 3. uses the stretched domain in the ozark example 4. adds unit tests. Note that the user passes the target (dz_bottom, dz_top) desired, as a Tuple, or `nothing`, if no stretching is needed. We then create the ClimaCore.Meshes.GeneralizedExponentialStretching object in the domain constructor appropriately. Instead, the user could pass the stretching object itself, and the default could be `ClimaCore.Meshes.Uniform`. In a way this is nicer (it's not prescribed that they use Uniform or GeneralizedExponentialStretching, gives them all the options that ClimaCore develops in the future, no `if` statement in the constructor), but I felt this might be more confusing than supplying the target layer widths at the top and bottom. Happy to change if anyone feels strongly! Review checklist I have: - followed the codebase contribution guide: https://clima.github.io/ClimateMachine.jl/latest/Contributing/ - followed the style guide: https://clima.github.io/ClimateMachine.jl/latest/DevDocs/CodeStyle/ - followed the documentation policy: https://github.com/CliMA/policies/wiki/Documentation-Policy - checked that this PR does not duplicate an open PR. In the Content, I have included - relevant unit tests, and integration tests, - appropriate docstrings on all functions, structs, and modules, and included relevant documentation. ---- - [X] I have read and checked the items on the review checklist. Co-authored-by: kmdeck <kdeck@caltech.edu>
mitraA90
pushed a commit
that referenced
this issue
Dec 22, 2023
310: Domain stretching r=kmdeck a=kmdeck ## Purpose Allows for variable domain stretching (finer resolution at the top of the domain compared to the bottom). Whenever we need dz in source code, we obtain it using the function `get_Δz`, which uses ClimaCore to extract the `dz` values. so we do not make the assumption of uniform spacing in src code. (we do in some tests, but that seems OK if those tests use uniform spacing.) ## To-do Future PRs: fix piracy and add the aqua test back in: #277 #311 ## Content 1. Adds domain stretching to all domains with a z coordinate. 2. Renames height to depth for the SphericalShell domain 3. uses the stretched domain in the ozark example 4. adds unit tests. Note that the user passes the target (dz_bottom, dz_top) desired, as a Tuple, or `nothing`, if no stretching is needed. We then create the ClimaCore.Meshes.GeneralizedExponentialStretching object in the domain constructor appropriately. Instead, the user could pass the stretching object itself, and the default could be `ClimaCore.Meshes.Uniform`. In a way this is nicer (it's not prescribed that they use Uniform or GeneralizedExponentialStretching, gives them all the options that ClimaCore develops in the future, no `if` statement in the constructor), but I felt this might be more confusing than supplying the target layer widths at the top and bottom. Happy to change if anyone feels strongly! Review checklist I have: - followed the codebase contribution guide: https://clima.github.io/ClimateMachine.jl/latest/Contributing/ - followed the style guide: https://clima.github.io/ClimateMachine.jl/latest/DevDocs/CodeStyle/ - followed the documentation policy: https://github.com/CliMA/policies/wiki/Documentation-Policy - checked that this PR does not duplicate an open PR. In the Content, I have included - relevant unit tests, and integration tests, - appropriate docstrings on all functions, structs, and modules, and included relevant documentation. ---- - [X] I have read and checked the items on the review checklist. Co-authored-by: kmdeck <kdeck@caltech.edu>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Remove the extra method defined in src/shared_utilties/ntuple_utils.jl (in fact, remove this file completely)
see discussion here:#263 with @charleskawczynski .
The text was updated successfully, but these errors were encountered: