Use new CV capabilities via arraycontext #355

MTCam · 2021-05-22T22:52:55Z

It is regretful this is such a giant haul, but it turns out this was something real hard to do incrementally.

inducer

Thanks so much for taking this on, this is great! Two thoughts from a quick scroll below.

inducer · 2021-05-22T23:37:31Z

mirgecom/fluid.py

@@ -313,6 +312,30 @@ def join_conserved(dim, mass, energy, momentum,
    return result


+def create_conserved(dim, mass, energy, momentum, species_mass=None):


Why is this needed? Couldn't we just call the ConservedVars constructor?

Can we overload the constructor and retain the shape and structure validation?

Fair point!

There's __post_init__ in a dataclass (docs), but revalidating on every object construction is an easy way to get really slow code. I'm warming to having an explicit function that does this. I've tended to call these make_xxx. (examples: make_kernel in loopy, make_xxx in pytato)

I hope this helps a little. (d012fba)

mirgecom/euler.py

inducer · 2021-05-22T23:49:31Z

And: I'm super supportive of trying to get this in super ASAP, to avoid a huge conflict nightmare.

MTCam · 2021-05-23T03:10:11Z

mirgecom/flux.py

+    return flux_avg.join() @ normal - 0.5*lam*(cv_tpair.ext.join()
+                                               - cv_tpair.int.join())


These joins are after:

return flux_avg @ normal - 0.5*lam*(cv_tpair.ext - cv_tpair.int)

gave this error:

E ValueError: matmul: Input operand 0 does not have enough dimensions (has 0, gufunc core with signature (n?,k),(k,m?)->(n?,m?) requires 1)

After using the "join" of flux_avg:

return flux_avg.join() @ normal - 0.5*lam*(cv_tpair.ext - cv_tpair.int)

this error:

E TypeError: unsupported operand type(s) for *: 'DOFArray' and 'ConservedVars'

Great question! inducer/arraycontext#12

As of inducer/arraycontext#15, this should now be doable. Note that you may need to tweak some of the arguments to with_container_arithmetic. See the tests.

(bc09408)

…raycontext#15).

MTCam · 2021-06-09T19:50:16Z

Ok, I've spent some time rolling these changes all the way up the Y1 tower - and everything only improves with it. I still regret this PR is so giant, but the changes are mostly trivial - and where they are not trivial they improve the code.

Hopefully not a terribly painful review.

inducer

Thanks, this looks great! A few notes below, otherwise good to go. FWIW, this was pretty painless to review.

mirgecom/simutil.py

test/test_bc.py

test/test_filter.py

test/test_euler.py

test/test_init.py

MTCam · 2021-06-10T15:10:29Z

Narrowly avoided a gitastrophy.

MTCam added 7 commits May 21, 2021 18:59

Do no harm with these decorators

d65ff1d

Document the array context getter

735f440

Huck the fluid infrastucture up to new CV capabilities.

ab2f8b2

Clean up straggilng interface differences

baa14d6

Update autoignition example to use new CV.

9bc2e8b

Fix up CV pair issue in euler operator

1b23a2c

Merge branch 'main' into cv-actx

1b8e033

inducer reviewed May 22, 2021

View reviewed changes

MTCam commented May 23, 2021

View reviewed changes

MTCam added 3 commits May 22, 2021 22:42

Fix some lingering interface tangle in integrated Euler tests

b5db033

Return the correct type for chem source

bb1cc1f

Merge branch 'cv-actx' into huck-up-to-cv

22fe30f

inducer mentioned this pull request May 23, 2021

Container-over-container broadcasting, broadcasting rules inducer/arraycontext#12

Closed

MTCam added 4 commits May 23, 2021 00:03

Bandage up partition boundary handling until inducer/grudge#97

b1d4021

Rename (and refactor a bit) create_conserved -> make_conserved.

d012fba

Fix some really ugly formatting in euler_operator

6639435

Update acoustic pulse init api call site.

86ec154

MTCam added the enhancement New feature or request label May 23, 2021

MTCam added 2 commits May 23, 2021 15:50

Merge branch 'main' into cv-actx

4f851b0

Merge branch 'cv-actx' into mrg-cv-actx

b669013

This was referenced May 24, 2021

Don't break Y1-Production with impactful changes to main #361

Closed

Use new CV - Navier-Stokes #365

Merged

MTCam added 5 commits May 27, 2021 20:14

Merge branch 'main' into cv-actx

2ebfb1e

Merge upstream.

3cd1e4e

Update to use better broadcast rules for array containers (inducer/ar…

1cb9ff4

…raycontext#15).

Merge branch 'cv-actx' into huck-up-to-cv

2fddc83

Update to use better broadcast rules for array containers (inducer/ar…

bc09408

…raycontext#15).

Base automatically changed from cv-actx to main June 5, 2021 05:21

Merge branch 'main' into huck-up-to-cv

1269f71

MTCam added 2 commits June 8, 2021 07:43

Merge branch 'main' into huck-up-to-cv

5e8f10f

Remove some crusty comments

eaa9e6c

MTCam marked this pull request as ready for review June 9, 2021 19:48

inducer approved these changes Jun 9, 2021

View reviewed changes

MTCam added 4 commits June 9, 2021 22:09

Remove crusty comments

27b13d1

Merge branch 'main' into huck-up-to-cv

54bd53e

Tweak slightly to partially address a review comment.

af33c88

Merge branch 'dumb' into huck-up-to-cv

82b355d

MTCam merged commit 1f8b3cb into main Jun 10, 2021

MTCam deleted the huck-up-to-cv branch June 10, 2021 23:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use new CV capabilities via arraycontext #355

Use new CV capabilities via arraycontext #355

MTCam commented May 22, 2021

inducer left a comment

inducer May 22, 2021

MTCam May 23, 2021

inducer May 23, 2021

MTCam May 23, 2021

inducer commented May 22, 2021 •

edited

Loading

MTCam May 23, 2021

inducer May 23, 2021

inducer May 29, 2021

MTCam Jun 1, 2021

MTCam commented Jun 9, 2021

inducer left a comment

MTCam commented Jun 10, 2021

		@@ -313,6 +312,30 @@ def join_conserved(dim, mass, energy, momentum,
		return result


		def create_conserved(dim, mass, energy, momentum, species_mass=None):

		return flux_avg.join() @ normal - 0.5lam(cv_tpair.ext.join()
		- cv_tpair.int.join())

Use new CV capabilities via arraycontext #355

Use new CV capabilities via arraycontext #355

Conversation

MTCam commented May 22, 2021

inducer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

inducer commented May 22, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MTCam commented Jun 9, 2021

inducer left a comment

Choose a reason for hiding this comment

MTCam commented Jun 10, 2021

inducer commented May 22, 2021 •

edited

Loading