illinois-ceesd / mirgecom

MIRGE-Com is the workhorse simulation application for the Center for Exascale-Enabled Scramjet Design at the University of Illinois.
Other
11 stars 19 forks source link

Fix issues with `==` in tests and update to work with numpy 1.25 #917

Closed majosm closed 1 year ago

majosm commented 1 year ago

We seem to have some bugs in our tests that were uncovered by the changes in object array behavior in numpy 1.25. Specifically, in a few places we assert on the equality of ConservedVars objects:

            assert ff_bndry_state.cv == exp_ff_cv

which looks reasonable enough, but upon inspection:

ff_bndry_state.cv == exp_ff_cv=ConservedVars(mass=DOFArray((cl.TaggableCLArray([[1],
       [1]], dtype=int8),)), energy=DOFArray((cl.TaggableCLArray([[1],
       [1]], dtype=int8),)), momentum=False, species_mass=array([], dtype=bool))
bool(ff_bndry_state.cv == exp_ff_cv)=True

i.e., == here is doing some strange things, and CV's boolean conversion is questionable. The discretization here is the boundary of a 1D mesh, so it looks like most of the components are doing elementwise equality, except for momentum.

If I understand correctly, ConservedVars gets its __eq__ operator from arraycontext's with_container_arithmetic decorator. @inducer Is the equality operator resulting from this decorator intended to do elementwise comparison? If so, is there a way we can make it work for array containers containing object arrays? Or do we just need to disable == for ConservedVars, as I've attempted to do here?

Questions for the review:

inducer commented 1 year ago

This is related, and I think the upshot is that == for array containers with object arrays is no longer a thing, with actx.np.equal(..., ...) the most plausible replacement.