Skip to content
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

istream support for cfloat #339

Open
davidmallasen opened this issue May 23, 2023 · 6 comments
Open

istream support for cfloat #339

davidmallasen opened this issue May 23, 2023 · 6 comments

Comments

@davidmallasen
Copy link
Contributor

Hello,

When reading numbers from a file to the fp16 format using std::ifstream, I'm getting an error:

undefined reference to `std::istream& sw::universal::operator>><16u, 5u, unsigned short, true, false, false>(std::istream&, sw::universal::cfloat<16u, 5u, unsigned short, true, false, false>&)'

I think this could be related to

Any insights on this? If this would be hard to implement, how could I hack the reading fp16 numbers from a file?

Thanks!

@ghost
Copy link

ghost commented May 23, 2023

What is the format of the FP16 in the file?

WIthin Universal we have been serializing using a hex format that carries the configuration of the type as prefix. Problem with that is that it doesn't work efficiently for matrix dumps as the format prefix should be set at the matrix level. That hurdle we have yet to cross.

Any preference?

@davidmallasen
Copy link
Contributor Author

Text. For now I'm reading into a double and then doing the casting down to a fp16 manually, so it's not a blocking issue, but it would be a nice addition.

@ghost
Copy link

ghost commented May 23, 2023

can you give me an example of the text you are getting in? For an FP16, we can just leverage the default double input and then use the assignment operator to cast to the underlying cfloat<>

@ghost
Copy link

ghost commented May 23, 2023

@davidmallasen I added the istream operator>> for cfloat in this commit 487c6ad

it takes the short cut of using the native operator>>(istream&, double) and then doing the conversion. The prototype was also incorrect, so fixed that too. It is in a new branch v3.70. If this works for you I can quickly merge it into main if needed.

@davidmallasen
Copy link
Contributor Author

I tested it and it works for me. Thanks @theo-lemurian !

@Ravenwater
Copy link
Contributor

TBD: we need to expand this to the big cfloats through a native implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants