search

Axect / Peroxide

Rust numeric library with high performance and friendly syntax
https://crates.io/crates/peroxide
Apache License 2.0
533 stars 32 forks source link

Support Complex number #35

Closed Axect closed 1 week ago

Axect commented 3 years ago

Still peroxide supports only real number. But for many cases, complex number, vector & matrix should be required.

GComitini commented 6 months ago

Hi @Axect. I am interested in this feature. At the moment I am using a fork of peroxide for my research which supports complex integration (by which I mean integration of complex functions of a real variable). My implementation, though working, is trivial (I wrote it without giving it much thought), so I don't believe it is upstreamable, but I am willing to work on a more suitable implementation that I can submit to you if you give me directions on what you would expect. To abandon my fork I would need full support of complex numbers, but since - as I said - complex integration is all I am using at the moment, this would be a great starting point for me. Best.

Axect commented 6 months ago

Hi @GComitini. Thank you for your interest in the complex number support for Peroxide. I appreciate your willingness to contribute to the project.

Currently, Peroxide doesn't have any support for complex numbers, but I do have plans to add this feature in the future. For matrices and vectors, I'm considering implementing separate CMatrix and CVector types to handle complex numbers.

However, for other operations like root finding, solving differential equations, and numerical integration, I don't have a clear idea yet on how to best implement complex number support. I'm open to suggestions and ideas from the community.

One approach I'm considering is to create a separate complex module within Peroxide and implement all complex number-related functions and structs there. (e.g. src/complex/matrix.rs, src/complex/integral.rs, and etc.) This would provide a clear separation between real and complex number functionality, improve code organization, and allow users to opt-in to complex number support when needed.

As a first step, I plan to create the complex module and implement some basic complex number-related functions. It would be greatly appreciated if you could contribute by implementing the complex integration functionality you're currently using within this module, perhaps in a file named integral.rs.

Regarding the namespace, although I intend to keep it separate from fuga and prelude, it might be a good idea to differentiate the names slightly for potential compatibility reasons. For example, using names like CMatrix and cintegral. However, this is still in the conceptual stage, so if you have any better ideas, please feel free to share them.

Implementing complex number support incrementally, starting with the most essential functions and gradually expanding, would be a pragmatic approach. This would also allow for better testing and refinement of the complex number implementations.

I would be happy to collaborate with you and other interested contributors to design and implement complex number support in Peroxide. We can discuss ideas, share experiences, and work together to create a robust and user-friendly complex number module.

Please feel free to share your thoughts, suggestions, or any specific ideas you have in mind. I'm excited to work with the community to bring complex number support to Peroxide.

Best regards, Axect

GComitini commented 6 months ago

Hi @Axect. I think creating a complex module within the main namespace of the crate is a good idea (I see you've already implemented it in the dev branch!). Something that needs to be sorted out, I think, is code duplication. For example, as you can see here, what I'm doing to compute complex integrals is basically copy-pasting real functions and changing their return type (an exception would be complex_newton_cotes_quadrature, where I need to compute the Lagrange polynomial twice, one for the real part of the integrand and one for its imaginary part). While this is not a problem in the context of my fork, I think duplicating code this way may soon become not so manageable within peroxide itself. If you're fine with having such a duplication (at least until a better solution is found) then I may already contribute with a src/complex/integral.rs. Otherwise I think we should first figure out some kind of generics, such as a common trait, so that functions are implemented once and for all when possible.

Axect commented 6 months ago

Hi @GComitini,

Thanks for suggesting the use of generics to reduce duplication in the complex number integration functions. I appreciate your input.

I've been considering how to make functions like newton_cotes_quadrature generic for both real and complex numbers. The main challenge I see is with the lagrange_polynomial function, which would need to support complex coefficients in the Polynomial struct. I'm not sure if this would be beneficial or if it could impact performance. What are your thoughts?

If there's a way to implement generics without major changes to existing structs, I would be open to that. It could be a good solution.

No definite decision has been made yet, and I'm always open to suggestions and discussions. Your ideas are highly valued and appreciated.

Thank you again for your contribution. I value your thoughts and look forward to continuing our discussion.

soumyasen1809 commented 1 month ago

@Axect Hi, I am new to Rust and would also want to take a shot at this item. I see that you have already implemented Vectors for Complex in the dev branch. Can I work on the Matrix support for Complex?

Also, would you please update which items in this list are done, and which are still remaining/newly added?

GComitini commented 1 month ago

@Axect I'm still interested too, all the more because integrating my changes into mainline peroxide would allow me to publish my library on crates.io without needlessly publishing a peroxide fork. Unfortunately, the last time I tried to write a (very long) reply to your last message my battery died and I lost everything I wrote, then I couldn't find the time to rewrite it from scratch.

The TL;DR was: I don't think lagrange_polynomial must support complex coefficients. If all we want to implement is integration of a complex function of a real variable (which by the way can be easily extended later on to integration over a path on the complex plane), then the integrand can be split as f(x)=u(x)+iv(x), where u and v are real functions of a real variable. Thus all lagrange_polynomial must do is spit out two real polynomials, one obtained from the real part of f (u), and the other one from its imaginary part (v). This is what I'm doing in my fork.

Axect commented 1 month ago

Hello, @soumyasen1809

Thank you for your interest. I've checked off the items related to Vector implementation in the issue list above, as they have all been completed.

Working on Complex Matrix support is an excellent idea. However, since performance is crucial, I suggest approaching it in the following ways:

  1. Currently, the pure Rust portion relies on the matrixmultiply crate for real matrix operations. This crate is exceptionally performant, so it would be great if you could explore ways to utilize it for complex matrix operations as well.

  2. In the O3 feature, we use BLAS/LAPACK. If you're interested in BLAS and LAPACK integration, implementing complex matrix operations within this feature would be valuable.

  3. If you know of any superior algorithms, we always welcome direct implementations as long as they guarantee good performance.

Let me know if you need any clarification or have any questions about these suggestions. I'm looking forward to your contributions!

Axect commented 1 month ago

Thank you @GComitini for your detailed explanation and suggestion. I apologize for the delay in our discussion, and I appreciate your continued interest in this feature.

Your approach to handling complex integration by splitting the integrand into real and imaginary parts (f(x) = u(x) + iv(x)) is indeed a good idea. Now that we've separated the complex feature into its own namespace, the issue of namespace duplication is resolved. As you suggested, it seems we can proceed without needing to support complex coefficients in the lagrange_polynomial function, which simplifies our implementation.

I noticed you've submitted a pull request related to this feature. That's great progress! I think the next step would be for me to review your PR in detail. This will give us a concrete foundation to discuss the specific design aspects of the complex integration implementation.

As we go through the PR, we can address any particular implementation details or design choices that need further consideration. This approach will allow us to fine-tune the implementation and ensure it aligns well with the overall structure of Peroxide.

Thank you again for your valuable input and for taking the initiative to submit a PR. Your expertise in this area is greatly appreciated, and I look forward to working together on the details of this feature. Let's continue our discussion in the context of your pull request.

P.S. As it's been a while since our initial discussion, there might be some details I'm not remembering correctly. If you notice any inconsistencies or points that don't align with our previous conversations, please don't hesitate to point them out. I'm open to any corrections or clarifications you might have.

GComitini commented 1 month ago

P.S. As it's been a while since our initial discussion, there might be some details I'm not remembering correctly. If you notice any inconsistencies or points that don't align with our previous conversations, please don't hesitate to point them out. I'm open to any corrections or clarifications you might have.

Same for me! I apologize for the delay but I've been quite busy during the last months.

I noticed you've submitted a pull request related to this feature. That's great progress! I think the next step would be for me to review your PR in detail. This will give us a concrete foundation to discuss the specific design aspects of the complex integration implementation.

As we go through the PR, we can address any particular implementation details or design choices that need further consideration. This approach will allow us to fine-tune the implementation and ensure it aligns well with the overall structure of Peroxide.

Looking forward to the discussion. I marked the PR as a draft precisely because that's what it is: a proposal for an implementation. Especially because I'm fully aware that it makes largish changes (albeit non-functional: as I wrote in the MR description, one of my aims was to leave real integration untouched) to the integration module, feel free to disagree with the design choices, I have no specific commitment to them, and if you think of ways to improve them I'll be happy to hear them.

Let's continue our discussion in the context of your pull request.

Sure!

soumyasen1809 commented 1 month ago

@Axect Hi, I have an initial PR for this issue https://github.com/Axect/Peroxide/pull/71

Note that:

  1. This is still WIP and hence I have requested a merge with dev branch and NOT master.
  2. There is still quite a lot of code left to be implemented from matrix.rs.
  3. This is a starting point. Once merged, others can also jump in and start developing on top.
  4. I have added cgemm trait in the Cargo.toml which will be needed for GEMM computations using matrixmultiply crate.
  5. Tests are still lacking. However, for now, I have tried to add doctests where possible.
  6. Please also check the 2 comments in the PR (since there is a duplication in the trait done by me, which I will remove in the next PRs).

Let's merge in small batches of PRs, since this issue is a big one.

soumyasen1809 commented 1 month ago

@Axect Hi, I have the 2nd PR for this issue https://github.com/Axect/Peroxide/pull/73

Axect commented 1 month ago

Update on Issue #35: Support Complex Numbers

Dear team,

I wanted to provide an update on our progress with Issue #35 (Support Complex number) in the Peroxide project.

Current Status

Challenges and Decision

As we progressed, we encountered two main challenges:

  1. The scope of modifications became quite extensive.
  2. The branch structure became increasingly complex and intertwined.

Given these challenges, we've made the following decision:

Moving Forward

To maintain better organization and clarity in our development process, we propose the following approach for future work on this issue:

  1. For each new modification or feature related to complex number support: a. Create a new, focused branch from the current dev branch. b. Implement the specific modification or feature in this new branch. c. Once the work is complete, create a Pull Request (PR) to merge this branch back into the dev branch. d. After review and approval, merge the PR into dev.

  2. This approach will help us manage smaller, more manageable chunks of work and avoid the complications we encountered with the previous long-running branch.

We believe this strategy will lead to a more streamlined development process and easier code reviews. It will also allow for more frequent integration of new features and modifications into our main development branch.

Please let me know if you have any questions or concerns about this approach. Your feedback is always welcome as we continue to improve our project management and development workflows.

Thank you for your ongoing contributions to the Peroxide project.

Axect commented 1 week ago

Hello @GComitini and @soumyasen1809

Now, we publish v0.38.0 including complex and parallel features. Thanks again for your great contributions!

GComitini commented 1 week ago

Thanks @Axect! I was waiting for this to publish my own crate on crates.io :-)

Axect commented 1 week ago

@GComitini Congratulations on publishing qcd-sme! It's wonderful to see your work going live on crates.io. And thank you again for your valuable contributions - they've helped make our project even better. Looking forward to seeing more of your work!