Skip to content

implict term bug fix in Octave addScalarBC - #476

Open
bewagner1 wants to merge 2 commits into
csrc-sdsu:mainfrom
bewagner1:implicitAddScalar2D
Open

bewagner1 wants to merge 2 commits into
csrc-sdsu:mainfrom
bewagner1:implicitAddScalar2D

Conversation

@bewagner1

Copy link
Copy Markdown
Collaborator

What type of PR is this? (check all applicable)

  • Refactor
  • Feature
  • Bug Fix
  • Optimization
  • Example
  • Documentation

Description

Both addScalarBC2D and addScalarBC3D silently fail when there are implicit (linear) terms in the operator they are adding boundary conditions to.
The line [rows1,cols1,s1] = find(A(rowsbc1,:)) causes rows1 to be [1; 2; 3; 4; ...; numel(rowsbc1)] when there are non-zero terms present in A(rowsbc1,:). This results in the left, right, and top boundary conditions being incorrectly applied as the line A = A - sparse(rows1, cols1, s1, size(A,1), size(A,2)) does not correctly zero out the boundary rows.

This is fixed by replacing the problematic lines mentioned above with A(rows1,:) = sparse(numel(rows1),size(A,2));

addScalarBC1D does not have this problem.

Related Issues & Documents

QA Instructions, Screenshots, Recordings

Run the following code snippet with the current implementation and compare with the results from the fixed version presented below:

k = 2;
m = 10;
n = 11;
o = 12;
dx = 1 / m;
dy = 1 / n;
dz = 1 / 0;

dc = [1;1];
nc = [0;0];
v = [0;0];

D =  div(k, m, dx, dc, nc);
G = grad(k, m, dx, dc, nc);

numC = m + 2;
L = D * G - spdiags(5 * ones(numC, 1), 0, numC, numC);

figure; spy(L); title("1D Before")
[L0,~] = addScalarBC1D(L,zeros(numC,1),k,m,dx,dc,nc,v);
figure; spy(L0); title("1D After")

dc = [1;1;1;1];
nc = [0;0;0;0];
v = {zeros(n,1);zeros(n,1);zeros(m+2,1);zeros(m+2,1)};

D =  div2D(k,m,dx,n,dy,dc,nc);
G = grad2D(k,m,dx,n,dy,dc,nc);

numC = (m + 2) * (n + 2);
L = D * G - spdiags(5 * ones(numC,1), 0, numC, numC);

figure; spy(L); title("2D Before")
[L0,~] = addScalarBC2D(L,zeros(numC,1),k,m,dx,n,dy,dc,nc,v);
figure; spy(L0); title("2D After")

dc = [1;1;1;1;1;1];
nc = [0;0;0;0;0;0];
v = {zeros(o*n,1);zeros(o*n,1);zeros(o*(m+2),1);zeros(o*(m+2),1);zeros((n+2)*(m+2),1);zeros((n+2)*(m+2),1)};

D =  div3D(k,m,dx,n,dy,o,dz,dc,nc);
G = grad3D(k,m,dx,n,dy,o,dz,dc,nc);

numC = (m + 2) * (n + 2) * (o + 2);
L = D * G - spdiags(5 * ones(numC,1), 0, numC, numC);

figure; spy(L); title("3D Before")
[L0,~] = addScalarBC3D(L,zeros(numC,1),k,m,dx,n,dy,o,dz,dc,nc,v);
figure; spy(L0); title("3D After")
2db 2da 3db 3da

Keep-open request

  • I am requesting maintainer review for keep-open.

Reason:

Added/updated tests?

  • Yes
  • No, and this is why: Don't know how to write Octave test cases
  • I need help with writing tests

Read Contributing Guide and Code of Conduct

[optional] Are there any post deployment tasks we need to perform?

No

[optional] What gif best describes this PR or how it makes you feel?

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

addScalarBC2D fails with implicit terms

1 participant