Skip to content

Conversation

selvavm
Copy link
Contributor

@selvavm selvavm commented Apr 17, 2021

Made ggev functions as non-scalar functions and support for lapack-src v0.13

#27

@selvavm
Copy link
Contributor Author

selvavm commented Apr 18, 2021

I have also changed to argument case to lower. Check if it is okay? Other alternative is to ignore case

…lsame

Arguments are now in lower case

alpha in larfg is corrected as a scalar
@selvavm
Copy link
Contributor Author

selvavm commented Apr 18, 2021

@IvanUkhov - I was facing issue with lsame and added a code similar to f.name=='lsame'. Is there better way to handle this?

bin/generate.py Outdated
@@ -190,15 +201,15 @@ def format_body(f):
def format_header_arguments(f):
s = []
for arg in f.args:
s.append('{}: {}'.format(arg[0], translate_argument(*arg, f=f)))
s.append('{}: {}'.format(arg[0].lower(), translate_argument(*arg, f=f)))
Copy link
Member

Choose a reason for hiding this comment

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

Why is this needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added tolower for all the arguments because there were too many warnings. I am also not sure if this is needed here as well.

If you dislike the idea of tolower, then we should add the ignore.

Copy link
Member

Choose a reason for hiding this comment

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

This is a good thing to lowercase. In this high-level crate, we want to see Rust-friendly names.

It actually made me realize that the logic in generate.py is broken, as it assumes everything is lowercase. I’ll push a fix.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. And let me know if you need me to change something. Waiting for your publish to crates.io

Copy link
Member

Choose a reason for hiding this comment

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

Done.

@IvanUkhov
Copy link
Member

I propose to just skip lsame. Not sure if anybody would want to call it. Please pull and take a look.

@IvanUkhov
Copy link
Member

It’s getting too big. I’ll merge this, and any other problems will be solved in separate pull requests.

@IvanUkhov IvanUkhov merged commit 0276f89 into blas-lapack-rs:master Apr 18, 2021
@selvavm
Copy link
Contributor Author

selvavm commented Apr 18, 2021

Thanks for merging.

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

Successfully merging this pull request may close these issues.

2 participants