-
Notifications
You must be signed in to change notification settings - Fork 107
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Save fields as 32 bit floats #6038
Conversation
@@ -50,7 +50,7 @@ def export_roff( | |||
|
|||
def import_roff( | |||
filelike: Union[TextIO, BinaryIO, _PathLike], name: Optional[str] = None | |||
) -> np.ma.MaskedArray[Any, np.dtype[np.double]]: | |||
) -> np.ma.MaskedArray[Any, np.dtype[np.float32]]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@JHolba
Can I assume that the code that does the import does the right thing and loads 32bit floats as that is what is stored in .roff
files?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like that should be returning 32bit as long as the file is 32bit.
the typing is currently incorrect.
it will return a 64 bit array if the file is 64 bit though.
@eivindjahren
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added an explicit conversion to 32bit in case it is 64bit as xtgeo
can create 64bit .roff
files.
Ran Drogon and seems to work. storage: runpath: |
RMS and ECLIPSE use 32 bit floats so there's no point in using 64 bits.
Codecov Report
@@ Coverage Diff @@
## main #6038 +/- ##
==========================================
+ Coverage 82.56% 82.58% +0.01%
==========================================
Files 352 352
Lines 21897 21899 +2
Branches 826 826
==========================================
+ Hits 18080 18085 +5
+ Misses 3518 3516 -2
+ Partials 299 298 -1
... and 2 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
lgtm |
Issue
Resolves #5965
Pre review checklist
Pre merge checklist
guidelines.