Extend `flattened_tup_chain` capability for diagnostics, update VariableTemplates #1929

charleskawczynski · 2021-01-15T16:36:39Z

Description

This PR:

Removes the CPU getindex(x, i::Int) fallback (# Somehow needed for GPU... was basically a TODO), so our CPU/GPU failures should be more nicely in sync. Interestingly, the getindex(x, i::Int) was hit on the GPU but worked in certain situations and not others. I've accordingly wrapped indexes with Val (in EDMF-related files)
Simplifies the nested intertwined getproperty/getindex flow control, which previously used if-else and now only uses dispatch
Split some methods to avoid isa calls in getindex

And the new feature:

flattened_tup_chain now has two modes of operation: one that retains arrays (RetainArr) and flatten arrays (FlattenArr).
flattened_named_tuple also has this option. Here is a demo (where v::Vars and vars_state(...) = @vars(x::SVector{3, FT})):

    fnt = flattened_named_tuple(v, RetainArr())
    @test fnt.x == Float32[1.0, 2.0, 3.0]

    fnt = flattened_named_tuple(v, FlattenArr())
    @test fnt.x_1 == 1.0
    @test fnt.x_2 == 2.0
    @test fnt.x_3 == 3.0

I've kept the old functionality (with RetainArr), as this may be useful if we want to automate traversing variables (e.g., #1921, in which case we'll need to work with the vectors)

This also flattens the single stack diagnostics, which will allow us to more easily automate the conversion between dons_arr and diag_arr cc @ilopezgp.

This also cleans up some of the edmf QA script. ⏳

Code follows the style guidelines OR N/A.
Unit tests are included OR N/A.
Code is exercised in an integration test OR N/A.
Documentation has been added/updated OR N/A.

charleskawczynski · 2021-01-15T19:02:56Z

bors r+

@vars

1929: Extend `flattened_tup_chain` capability for diagnostics, update VariableTemplates r=charleskawczynski a=charleskawczynski ### Description This PR: - Removes the CPU `getindex(x, i::Int)` fallback (`# Somehow needed for GPU...` was basically a TODO), so our CPU/GPU failures should be more nicely in sync. Interestingly, the `getindex(x, i::Int)` was hit on the GPU but worked in certain situations and not others. I've accordingly wrapped indexes with `Val` (in EDMF-related files) - Simplifies the nested intertwined `getproperty`/`getindex` flow control, which previously used `if-else` and now only uses dispatch - Split some methods to avoid `isa` calls in `getindex` And the new feature: - `flattened_tup_chain` now has two modes of operation: one that retains arrays (`RetainArr`) and flatten arrays (`FlattenArr`). - `flattened_named_tuple` also has this option. Here is a demo (where `v::Vars` and `vars_state(...) = @vars(x::SVector{3, FT})`): ```julia fnt = flattened_named_tuple(v, RetainArr()) @test fnt.x == Float32[1.0, 2.0, 3.0] fnt = flattened_named_tuple(v, FlattenArr()) @test fnt.x_1 == 1.0 @test fnt.x_2 == 2.0 @test fnt.x_3 == 3.0 ``` I've kept the old functionality (with `RetainArr`), as this may be useful if we want to automate traversing variables (e.g., #1921, in which case we'll need to work with the vectors) This also flattens the single stack diagnostics, which will allow us to more easily automate the conversion between `dons_arr` and `diag_arr` cc @ilopezgp. This also cleans up some of the edmf QA script. ⏳ Co-authored-by: Charles Kawczynski <kawczynski.charles@gmail.com>

bors · 2021-01-15T19:49:51Z

Build failed:

buildkite/climatemachine-ci

charleskawczynski · 2021-01-16T00:53:07Z

bors r+

bors · 2021-01-16T02:50:59Z

Build succeeded:

@yairchn

1931: Fix NamedTuple keys, add error handling r=charleskawczynski a=charleskawczynski ### Description After #1929, there is some mismatch with the keys and values returned from `flattened_named_tuple` and thus `single_stack_diagnostics` (e.g., IIRC, mixing length). cc @yairchn @ilopezgp. This PR should fix this mismatch. Specifically, adding ```julia length(keys_) == length(vals) || error("key-value mismatch") ``` to `flattened_named_tuple` errors on master due to improper handling of `SHermitianCompact` and `Diagonal` arrays. This required some changes / additions to `flattened_nt_vals`. Co-authored-by: Charles Kawczynski <kawczynski.charles@gmail.com>

@yairchn

1931: Fix NamedTuple keys, add error handling r=charleskawczynski a=charleskawczynski ### Description After #1929, there is some mismatch with the keys and values returned from `flattened_named_tuple` and thus `single_stack_diagnostics` (e.g., IIRC, mixing length). cc @yairchn @ilopezgp. This PR should fix this mismatch. Specifically, adding ```julia length(keys_) == length(vals) || error("key-value mismatch") ``` to `flattened_named_tuple` errors on master due to improper handling of `SHermitianCompact` and `Diagonal` arrays. This required some changes / additions to `flattened_nt_vals`. Co-authored-by: Charles Kawczynski <kawczynski.charles@gmail.com>

@yairchn

1931: Fix NamedTuple keys, add error handling r=charleskawczynski a=charleskawczynski ### Description After #1929, there is some mismatch with the keys and values returned from `flattened_named_tuple` and thus `single_stack_diagnostics` (e.g., IIRC, mixing length). cc @yairchn @ilopezgp. This PR should fix this mismatch. Specifically, adding ```julia length(keys_) == length(vals) || error("key-value mismatch") ``` to `flattened_named_tuple` errors on master due to improper handling of `SHermitianCompact` and `Diagonal` arrays. This required some changes / additions to `flattened_nt_vals`. Co-authored-by: Charles Kawczynski <kawczynski.charles@gmail.com>

@yairchn

1931: Fix NamedTuple keys, add error handling r=charleskawczynski a=charleskawczynski ### Description After #1929, there is some mismatch with the keys and values returned from `flattened_named_tuple` and thus `single_stack_diagnostics` (e.g., IIRC, mixing length). cc @yairchn @ilopezgp. This PR should fix this mismatch. Specifically, adding ```julia length(keys_) == length(vals) || error("key-value mismatch") ``` to `flattened_named_tuple` errors on master due to improper handling of `SHermitianCompact` and `Diagonal` arrays. This required some changes / additions to `flattened_nt_vals`. Co-authored-by: Charles Kawczynski <kawczynski.charles@gmail.com>

@yairchn

1931: Fix NamedTuple keys, add error handling r=charleskawczynski a=charleskawczynski ### Description After #1929, there is some mismatch with the keys and values returned from `flattened_named_tuple` and thus `single_stack_diagnostics` (e.g., IIRC, mixing length). cc @yairchn @ilopezgp. This PR should fix this mismatch. Specifically, adding ```julia length(keys_) == length(vals) || error("key-value mismatch") ``` to `flattened_named_tuple` errors on master due to improper handling of `SHermitianCompact` and `Diagonal` arrays. This required some changes / additions to `flattened_nt_vals`. Co-authored-by: Charles Kawczynski <kawczynski.charles@gmail.com>

@yairchn

1931: Fix NamedTuple keys, add error handling r=charleskawczynski a=charleskawczynski ### Description After #1929, there is some mismatch with the keys and values returned from `flattened_named_tuple` and thus `single_stack_diagnostics` (e.g., IIRC, mixing length). cc @yairchn @ilopezgp. This PR should fix this mismatch. Specifically, adding ```julia length(keys_) == length(vals) || error("key-value mismatch") ``` to `flattened_named_tuple` errors on master due to improper handling of `SHermitianCompact` and `Diagonal` arrays. This required some changes / additions to `flattened_nt_vals`. Co-authored-by: Charles Kawczynski <kawczynski.charles@gmail.com>

@yairchn

1931: Fix NamedTuple keys, add error handling r=charleskawczynski a=charleskawczynski ### Description After #1929, there is some mismatch with the keys and values returned from `flattened_named_tuple` and thus `single_stack_diagnostics` (e.g., IIRC, mixing length). cc @yairchn @ilopezgp. This PR should fix this mismatch. Specifically, adding ```julia length(keys_) == length(vals) || error("key-value mismatch") ``` to `flattened_named_tuple` errors on master due to improper handling of `SHermitianCompact` and `Diagonal` arrays. This required some changes / additions to `flattened_nt_vals`. Co-authored-by: Charles Kawczynski <kawczynski.charles@gmail.com>

charleskawczynski requested a review from kpamnany January 15, 2021 16:36

charleskawczynski force-pushed the ck/variable_templates branch from e63207a to 592be54 Compare January 15, 2021 17:01

kpamnany approved these changes Jan 15, 2021

View reviewed changes

charleskawczynski force-pushed the ck/variable_templates branch from 592be54 to b0c5231 Compare January 15, 2021 19:01

Extend flattened_tup_chain, update VarTemplates

38ee22b

charleskawczynski force-pushed the ck/variable_templates branch from b0c5231 to 38ee22b Compare January 16, 2021 00:52

bors bot merged commit 8c57fb5 into master Jan 16, 2021

bors bot deleted the ck/variable_templates branch January 16, 2021 02:51

charleskawczynski mentioned this pull request Jan 17, 2021

Fix NamedTuple keys, add error handling #1931

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extend `flattened_tup_chain` capability for diagnostics, update VariableTemplates #1929

Extend `flattened_tup_chain` capability for diagnostics, update VariableTemplates #1929

charleskawczynski commented Jan 15, 2021 •

edited

Loading

charleskawczynski commented Jan 15, 2021

bors bot commented Jan 15, 2021

charleskawczynski commented Jan 16, 2021

bors bot commented Jan 16, 2021

Extend flattened_tup_chain capability for diagnostics, update VariableTemplates #1929

Extend flattened_tup_chain capability for diagnostics, update VariableTemplates #1929

Conversation

charleskawczynski commented Jan 15, 2021 • edited Loading

Description

charleskawczynski commented Jan 15, 2021

bors bot commented Jan 15, 2021

charleskawczynski commented Jan 16, 2021

bors bot commented Jan 16, 2021

Extend `flattened_tup_chain` capability for diagnostics, update VariableTemplates #1929

Extend `flattened_tup_chain` capability for diagnostics, update VariableTemplates #1929

charleskawczynski commented Jan 15, 2021 •

edited

Loading