Skip to content

Conversation

@brownbaerchen
Copy link
Contributor

@brownbaerchen brownbaerchen commented Jan 21, 2025

I had some trouble with round-off errors today and noticed that my numpy doesn't actually have quadruple precision. So np.float128 will raise an error on my machine. np.longdouble, on the other hand, is just regular double on my machine. This is extremely weird behaviour that I neither like nor understand. I switched out the quadruple precision things for the word version so I can run the IO stuff with double precision at least.
Notice that the numpy documentation says that np.longdouble is an alias for np.float128 on Linux x86_64 machines. So this fix applies to Mac and Windows users.

@brownbaerchen brownbaerchen requested a review from tlunet January 21, 2025 16:16
@tlunet
Copy link
Member

tlunet commented Jan 22, 2025

Interesting ... I'm always a bit confused on how numpy handle datatypes. In particular there is this thing :

import numpy as np
u = np.zeros(10, dtype=np.float64)
u.dtype == np.float64  # => True
u.dtype is np.float64  # => False

The way data types are handled can probably still be improved, however I think replacing np.float128 by np.longdouble will make the FieldsIO very platform dependent, and may lead to inconsistent file sizes (in particular if on some platform, np.longdouble are simply standart double precision numbers ...). The main advantage of keeping this size-based notation is that at least, it's explicit on how much bytes are used to store a field value.

I would rather see some import dependent definition, like this :

DTYPES = {
    0: np.float64,  # double precision
    1: np.complex128,
}
try:
    DTYPES.update({
        2: np.float128,  # quadruple precision
        3: np.complex256,
    })
except AttributeError:
    # Some warning on double precision not available on this platform
# And the same for single precision ...

@brownbaerchen
Copy link
Contributor Author

You're right, that is a better solution! I went with the warning only on debug level and added a bit more elaborate message when you try to access a datatype that is not there.

}
)
except AttributeError:
import logging
Copy link
Member

@tlunet tlunet Jan 22, 2025

Choose a reason for hiding this comment

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

Can logging not be a module import ? Local import with black formating makes this very ugly ... 🤡

@tlunet
Copy link
Member

tlunet commented Jan 22, 2025

Looks all good, thanks !

@pancetta
Copy link
Member

Excellent solution, thanks!

@pancetta pancetta merged commit c472a48 into Parallel-in-Time:master Jan 22, 2025
86 checks passed
@brownbaerchen brownbaerchen deleted the fieldsIO_fix branch March 31, 2025 08:14
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.

3 participants