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

JOSS paper: copy edits #153

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

jedbrown
Copy link

@jedbrown jedbrown commented Oct 2, 2024

This mostly fixes some citing/bib ([@label] is special syntax). I introduced the "JIT" acronym at first use, though it could be reworded elsewhere.

Is it worth mentioning libxsmm since your nano-gemm implementation is surprisingly competitive without JIT? I don't know how to square that with mediocre performance at very small sizes in the figures here.

Should the mention of sparse matrix support be updated to reflect that it exists (perhaps in immature form) while still being out of scope for this paper?

openjournals/joss-reviews#6099

@@ -60,7 +60,7 @@ such as data races and fatal use-after-free errors.

Rust also allows compatibility with the C ABI, allowing for simple interoperability
with C, and most other languages by extension. Once a design has been properly fleshed out,
we plan to expose a C API, along with bindings to other languages (Currently planned are C, C++, Python and Julia bindings).
we plan to expose a C API, along with bindings to other languages (C, C++, Python and Julia bindings are currently planned).
Copy link
Author

Choose a reason for hiding this comment

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

My understanding is that there are some R and Python packages that are already using Faer, though I don't think there are comprehensive bindings.

Copy link
Owner

Choose a reason for hiding this comment

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

yeah, i think there's people using it but i haven't really been following their progress

@jedbrown
Copy link
Author

jedbrown commented Oct 2, 2024

One other optional comment about Rust is its noalias by default, where C requires restrict and C++ doesn't even have that (in the standard). This is often a substantial performance benefit in numerical code (especially when using AD), and it's hard to safely use restrict to the same degree (also as evidenced by LLVM noalias bugs being exposed by Rust nalgebra).

@sarah-quinones
Copy link
Owner

thanks a bunch!

the figures are from before nanogemm was a thing. there have been some good perf improvements since then

i think mentioning the sparse algorithms can't hurt

one thing about the noalias part is that faer can't really make use of it, because matrices are implemented as pointers. there are some places where i convert to slices before i do the math to get noalias annotations, but rustc currently doesn't propagate the annotations to llvm in some cases

@jedbrown
Copy link
Author

jedbrown commented Oct 3, 2024

Would you mind updating the figures for nanogemm?

Regarding noalias, I was thinking as much of the caller's code, though I also wouldn't say a missed optimization in rustc/LLVM negates the benefit of making it safe to maximize noalias in the programming model (though tangible performance impact may be waiting for that compiler improvement). This may well be too far in the weeds for the paper.

@sarah-quinones
Copy link
Owner

i don't know if i still have the benchmark code. i lost some data on my pc recently, but i'll check if i have a backup somewhere

@sarah-quinones
Copy link
Owner

nvm i put it on github

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.

2 participants