-
Notifications
You must be signed in to change notification settings - Fork 29
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
PSRDADA block updates #194
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #194 +/- ##
==========================================
- Coverage 68.19% 67.09% -1.11%
==========================================
Files 65 67 +2
Lines 5515 6026 +511
==========================================
+ Hits 3761 4043 +282
- Misses 1754 1983 +229 ☔ View full report in Codecov by Sentry. |
if len(s) > hdrlen: | ||
raise RuntimeError("Header is too large for HDR_SIZE! Increase hdrlen") | ||
n_pad = hdrlen - len(s) | ||
return s + s_padding * n_pad |
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.
Is any encoding needed here for Py2 vs Py3?
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.
very good chance there is :/. I guess I should write a unit test
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 don't think we have anything that covers psrdata now. We probably don't even install psrdata to test the wrapping.
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'm looking back through the code here and I'm not convinced that encoding is needed here.
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.
The CI now installs PSRDADA and the Bifrost wrappers get generated. @telegraphic do you have a .dada file I can test with?
Tensor core arithmetic also accelerates 16 bit type data.
…On 2/14/23 9:49 AM, jaycedowell wrote:
In python/bifrost/blocks/psrdada.py
<#194 (comment)>:
> + Returns:
+ s (str): DADA header string with padding to hdrlen
+ """
+ s = "HDR_VERSION 1.0\n"
+ s+= "HDR_SIZE %i\n" % hdrlen
+ keys_to_skip = ('HDR_VERSION', 'HDR_SIZE')
+
+ # update parameters from bifrost tensor
+ if '_tensor' in hdr_dict.keys():
+ print(hdr_dict['_tensor'])
+ dtype = hdr_dict['_tensor']['dtype']
+ dtype_vals = {
+ 'cf32': { 'NBIT': '32', 'NDIM': '2' },
+ 'f32': { 'NBIT': '32', 'NDIM': '1' },
+ 'ci8': { 'NBIT': '8', 'NDIM': '2' },
+ 'i8': { 'NBIT': '8', 'NDIM': '1' } }
Are additional data types expected/allowed?
--------------------------------------------------------------------------------
|
I raised the notion of 16-bit to accommodate directly time-series output from
many-bit ADCs. i16 would do that. Quick examination suggests that uint16 is
recognized by CUDA7(?) but using it may not result in speed advantage over
4-byte integer math. Proper research required.
…On 2/15/23 1:58 AM, Danny Price wrote:
***@***.**** commented on this pull request.
--------------------------------------------------------------------------------
In python/bifrost/blocks/psrdada.py
<#194 (comment)>:
> + Returns:
+ s (str): DADA header string with padding to hdrlen
+ """
+ s = "HDR_VERSION 1.0\n"
+ s+= "HDR_SIZE %i\n" % hdrlen
+ keys_to_skip = ('HDR_VERSION', 'HDR_SIZE')
+
+ # update parameters from bifrost tensor
+ if '_tensor' in hdr_dict.keys():
+ print(hdr_dict['_tensor'])
+ dtype = hdr_dict['_tensor']['dtype']
+ dtype_vals = {
+ 'cf32': { 'NBIT': '32', 'NDIM': '2' },
+ 'f32': { 'NBIT': '32', 'NDIM': '1' },
+ 'ci8': { 'NBIT': '8', 'NDIM': '2' },
+ 'i8': { 'NBIT': '8', 'NDIM': '1' } }
There isn't really a strict definition AFAIK. There is no header keyword for
float/integer, but I think it's reasonable to add cf64/f64. Would need to
decide between i16/ci16 and f16/cf16 though
—
Reply to this email directly, view it on GitHub
<#194 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACL54MQIQ7KCUU3K6EQ33PDWXR5CFANCNFSM6AAAAAAS7I5RJ4>.
You are receiving this because you commented.Message ID:
***@***.***>
|
These updates are from https://github.com/Molonglo/bifrost, which is used for UTMOST-2D.
An alternative to this merge is to move PSRDADA functionality into a plugin.