Direct L2 norm computation with Psydac inner products (no NumPy conversion)

[anushkasinghh]

Use np.sqrt instead of np.ToArray which reduces the computation time while asserting tolerance.

Will solve issue #492 when merged

[codacy-production]

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation	Diff coverage
✅ +0.00%	✅ ∅

Coverage variation details

	Coverable lines	Covered lines	Coverage
Common ancestor commit (42b214e)	30762	18640	60.59%
Head commit (eb364e5)	61524 (+30762)	37278 (+18638)	60.59% (+0.00%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details

	Coverable lines	Covered lines	Diff coverage
Pull request (#514)	0	0	∅ (not applicable)

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

[yguclu]

@anushkasinghh If this PR is ready, you should request potential reviewers. In general you can choose psydac-dev and all developers in this team will be notified.

[campospinto]

The changes look good to me, but I don't find the PR title very clear. Instead of describing the changes as a refactor of to array, you should mention the direct computation of array l2 norms using psydac inner products avoiding conversions to numpy arrays. I guess that the point is to save time, did you observe some gain ?

[anushkasinghh]

I ran the tests before and after the change, and the times are almost the same — around 170 seconds in both cases. The small differences I saw (a few tenths of a second) are just normal fluctuations, so there’s no real speedup.

[jowezarek]

We want to test the interaction of a PowerLinearOperator with a InverseLinearOperator here, so we have to check whether bi1 and bi2 have been computed correctly.

Suggested change

	bi12 = B_ILO @ u
	bi22 = B_ILO @ bi12
	assert norm2((B @ bi12) - u) < tol
	assert norm2((B @ bi22) - bi12 ) < error_est * tol
	assert norm2((B @ bi1) - u) < tol
	assert norm2((B @ B @ bi2) - u) < error_est * tol

[jowezarek]

Same thing here

Suggested change

	si12 = S_ILO @ v
	si22 = S_ILO @ si12
	assert norm2((S @ si12) - v) < tol
	assert norm2( (S @ si22) - si12 ) < error_est * tol
	assert norm2((S @ si1) - v) < tol
	assert norm2((S @ S @ si22) - si2) < error_est * tol

[jowezarek]

Suggested change

	assert norm2(v- i0) < 1e-10
	assert norm2(v - i1) <1e-10
	assert norm2(v - i2) < 1e-10
	assert norm2(v - i0) < 1e-10
	assert norm2(v - i1) < 1e-10
	assert norm2(v - i2) < 1e-10

[jowezarek]

Suggested change

	assert norm2((u_approx - u)) < tol
	assert norm2( (v_approx - v)) < tol
	assert norm2((u_approx - u)) < tol
	assert norm2((v_approx - v)) < tol

[jowezarek]

Suggested change

	assert norm2((S @ xs_cg - v)) < tol
	assert norm2( (S @ xs_pcg - v)) < tol
	assert norm2((S @ xs_bicg - v)) < tol
	assert norm2((S @ xs_cg - v)) < tol
	assert norm2((S @ xs_pcg - v)) < tol
	assert norm2((S @ xs_bicg - v)) < tol

[jowezarek]

Suggested change

	assert (norm2( y1_1- y1_2 ) < 1e-10) & (norm2( y1_2 - y1_3) < 1e-10)
	assert (norm2( y2_1 - y2_2 ) < 1e-10) & (norm2( y2_2 - y2_3) < 1e-10)
	assert (norm2(y1_1 - y1_2) < 1e-10) & (norm2(y1_2 - y1_3) < 1e-10)
	assert (norm2(y2_1 - y2_2) < 1e-10) & (norm2(y2_2 - y2_3) < 1e-10)

[jowezarek]

Hey @anushkasinghh, I added a few suggestions. Most of them are just cosmetics, but the PowerLinearOperator and InverseLinearOperator interaction test needed a proper change.
Other than that, this PR looks good to me. It will probably not decrease the test run time, but it's good practice to avoid unnecessary toarray calls.