Skip to content
This repository has been archived by the owner on Mar 1, 2023. It is now read-only.

Enable dims argument for weighted norms #590

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

simonbyrne
Copy link
Member

Previously this threw an error

@simonbyrne
Copy link
Member Author

bors r+

bors bot added a commit that referenced this pull request Dec 14, 2019
590: Enable dims argument for weighted norms r=simonbyrne a=simonbyrne

Previously this threw an error

Co-authored-by: Simon Byrne <[email protected]>
@bors
Copy link
Contributor

bors bot commented Dec 14, 2019

Build failed

@codecov
Copy link

codecov bot commented Dec 14, 2019

Codecov Report

Merging #590 into master will increase coverage by 0.18%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #590      +/-   ##
=========================================
+ Coverage   68.02%   68.2%   +0.18%     
=========================================
  Files          82      82              
  Lines        5620    5618       -2     
=========================================
+ Hits         3823    3832       +9     
+ Misses       1797    1786      -11
Impacted Files Coverage Δ
src/Arrays/MPIStateArrays.jl 68.04% <100%> (+5.28%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a1bc4b2...ed464b6. Read the comment docs.

@simonbyrne simonbyrne force-pushed the sb/weighted-norm-dims branch from 1a8bd84 to 6f18f44 Compare December 14, 2019 04:44
@simonbyrne
Copy link
Member Author

bors try

bors bot added a commit that referenced this pull request Dec 14, 2019
@bors
Copy link
Contributor

bors bot commented Dec 14, 2019

try

Build failed

Previously this threw an error
@simonbyrne simonbyrne force-pushed the sb/weighted-norm-dims branch from 6f18f44 to ed464b6 Compare December 14, 2019 05:03
@simonbyrne
Copy link
Member Author

bors r+

bors bot added a commit that referenced this pull request Dec 14, 2019
590: Enable dims argument for weighted norms r=simonbyrne a=simonbyrne

Previously this threw an error

Co-authored-by: Simon Byrne <[email protected]>
@bors
Copy link
Contributor

bors bot commented Dec 14, 2019

Build failed

@@ -488,7 +488,7 @@ function weighted_norm_impl(Q::SubArray{FT, N, A}, W, ::Val{p}, dims::Colon) whe
if isfinite(p)
accum += W[i, 1, k] * abs(Q[i, j, k]) ^ p
else
waQ_ijk = W[i, j, k] * abs(Q[i, j, k])
waQ_ijk = W[i, 1, k] * abs(Q[i, j, k])
Copy link
Member

Choose a reason for hiding this comment

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

Why is this being changed? (And why is the line above read with a 1 also: accum += W[i, 1, k] * abs(Q[i, j, k]) ^ p?)

Copy link
Member Author

Choose a reason for hiding this comment

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

weights are set up to have size(weights, 2) == 1

@blallen blallen added the DG Kernels For physics-agnostic issues / PRs mainly concerned with the DG implementation. label Jan 7, 2020
@bischtob
Copy link
Contributor

@simonbyrne, is this PR still relevant?

@simonbyrne
Copy link
Member Author

yes, we still need to fix it.

@test norm(QAW, Inf) ≈ norm(globalA, Inf)

if ArrayType == Array
# TODO: make this work with CuArrays
Copy link
Member

Choose a reason for hiding this comment

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

@blallen blallen mentioned this pull request Jul 23, 2020
18 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
DG Kernels For physics-agnostic issues / PRs mainly concerned with the DG implementation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants