Skip to content
This repository has been archived by the owner on Dec 6, 2024. It is now read-only.

Ksagiyam/attach dtype to nodes #327

Merged
merged 1 commit into from
Nov 20, 2024
Merged

Conversation

ksagiyam
Copy link
Contributor

@ksagiyam ksagiyam commented Nov 7, 2024

Follow up #317.

Attach dtype consistently to every node, and fix treatment of Zero()**{Min, Max}Value().

Add test in Firedrake: firedrakeproject/firedrake#3855

@ksagiyam ksagiyam force-pushed the ksagiyam/attach_dtype_to_nodes branch 7 times, most recently from 3e6a015 to 86ce77e Compare November 11, 2024 16:03
@ksagiyam ksagiyam marked this pull request as ready for review November 11, 2024 16:09
@ksagiyam ksagiyam force-pushed the ksagiyam/attach_dtype_to_nodes branch 2 times, most recently from 7cd2b19 to 50e938e Compare November 13, 2024 14:55
dham
dham previously requested changes Nov 13, 2024
Copy link
Member

@dham dham left a comment

Choose a reason for hiding this comment

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

just fix the "!=" to "is not" for types.

gem/gem.py Show resolved Hide resolved
@ksagiyam ksagiyam force-pushed the ksagiyam/attach_dtype_to_nodes branch 3 times, most recently from f9db049 to 3bd7499 Compare November 19, 2024 14:22
@ksagiyam
Copy link
Contributor Author

It turned out that object equal caused some pickling tests to fail:

import numpy as np 
import pickle 


obj1 = np.dtype(int) 
obj2 = np.dtype(int) 
print(obj1 == obj2)  # True 
print(obj1 is obj2)  # True 
obj1_pickled = pickle.dumps(obj1) 
obj1_loaded = pickle.loads(obj1_pickled) 
print(obj1_loaded == obj1)  # True 
print(obj1_loaded is obj1)  # False 

So probably we should just use == instead of object equal.

@ksagiyam ksagiyam force-pushed the ksagiyam/attach_dtype_to_nodes branch from 3bd7499 to db10fd7 Compare November 19, 2024 15:44
@dham dham dismissed their stale review November 20, 2024 17:07

I was wrong.

@dham dham merged commit 31cca14 into master Nov 20, 2024
4 checks passed
@dham dham deleted the ksagiyam/attach_dtype_to_nodes branch November 20, 2024 17:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants