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

Make fidimag great again #138

Open
wants to merge 50 commits into
base: master
Choose a base branch
from
Open

Make fidimag great again #138

wants to merge 50 commits into from

Conversation

rpep
Copy link
Member

@rpep rpep commented May 9, 2019

Not ready to merge; just for test running.

The motivation behind this:

  • Simplify building by creating a library (shared? static?) of the C/C++ code seperate to the Python interface, and automate Cython file discovery to make it much easier to develop new tools.

  • Reduce code duplication by wrapping C functions once and exposing via pxd.

  • Move more Python code into Cython.

  • Profile and find the slowest parts for realistic examples

  • Give better support for compiling against other libraries; i.e. MKL if available vs FFTW - Should we use CMake rather than Make to simplify discovery or leave it to the user?

Large commit - port Fidimag to build everything as C++ rather than C.
Why? Because this is Stage 1 of our complex plan to make Fidimag faster
and more readable, and part of this involves using Classes at the C++
level rather than at the Python level for energy terms in order to
reduce the amount of slow Python code.

Co-Authored-By: David Ignacio Cortee <[email protected]>
@rpep rpep requested a review from davidcortesortuno May 9, 2019 17:10
rpep and others added 19 commits May 9, 2019 18:29
…me functions to new file tools.py (not setuptools.py because module named this)
…math.cpp. Make some of these external C++ declarations available in other modules by using a Cython pxd file, vectormath.pxd.

Co-Authored-By: David Ignacio Cortee <[email protected]>
Co-Authored-By: David Ignacio Cortee <[email protected]>
…finitions of WIDE_PI constant with use of math library M_PIl

Co-Authored-By: David Ignacio Cortee <[email protected]>
@fangohr
Copy link
Member

fangohr commented May 13, 2019

The title of the merge request is hilarious. Keep going :)

@davidcortesortuno
Copy link
Collaborator

davidcortesortuno commented Jun 1, 2021

I'd make the RK4 arrays sequential to make the memory layout more compact, so something like (this won't compile I think, needs more thought)

class Integrator {
Integrator(size_t N) = 0; // No constructor so people can't use the base class
public:
...
};

The compiler gives me an error where the base class constructor is not virtual so I can't make = 0 to the constructor.

@rpep rpep closed this Oct 2, 2023
@davidcortesortuno
Copy link
Collaborator

We should not close this PR. But I'm not sure if someone would like to keep developing this full C backend, it's a lot of work :O

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.

3 participants