Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
/mfemPublic
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Okina GPU [okina]#631

Merged
camierjs merged 637 commits intomasterfromokina
Apr 12, 2019
Merged

Okina GPU [okina]#631

camierjs merged 637 commits intomasterfromokina
Apr 12, 2019

Conversation

camierjs
Copy link
Member

@camierjscamierjs commentedOct 13, 2018
edited by tzanio
Loading

See themfem-4.0-rc1 tasks in#813.

@tzaniotzanio modified the milestones:mfem-4.0,mfem-4.1Oct 16, 2018
Copy link
Contributor

@YohannDudouitYohannDudouit left a comment

Choose a reason for hiding this comment

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

I'm a really impressed by all the work done. It gives a good idea of all the different parts of the code that have to change to port the first set of functionalities of MFEM to GPU. From the MFEM core side, I think most of the changes are not too disruptive. I really like that the device/backend specific functions are exposed and a developer doesn't have to guess what has to be written, the level of abstraction is at a granularity where it is easy for a developer to understand what the backend functions have to do. I think l would feel confortable writing a backend in this code. However, I think the design should be pushed a little bit more in the direction of the separation of what is device specific and what is backend specific.
I would give the following definitions of device specific and backend specific:

  • Device functions are meant to be uniquely implemented for a specific device. If a better implementation is found for a device function, it should replace the current implementation. For instance, all backends should use the same implementation ofdot on a GPU (I think). Device functions assume that there exists a best implementation on a device usable for all the different use cases.
  • Backend functions are meant to have multiple implementations for a specific device. Backend specific functions are selected by the user and are expected to have different performance on different problem configurations.

Instead of having everything inkernels files, maybe having some indevice files and other inbackend files would help clarifying this. And showing how the differentbackend functions could be selected would be really interesting (amap<Backend, FunctionPointer>?).
Havingbackend files would also allow to directly see what needs to implemented by a backend.
Going even further in this design, I think it is important that abackend can only propose the implementation of a subset of all thebackend functions, and either select whichbackend to use when a backend function is not implemented, or use a default backend.

Regarding theArray,Vector classes, I would like to see if there is no impact on performance of the double pointer when some computations are achieved on the CPU. If there is a negative impact on performance, we might want to consider keeping themfem::Array andmfem::Vector classes as they are and make them compatible with the proposedArray andVector classes, they would have to be renamed toDeviceArray andDeviceVector (or something else). These classes would have a more limited set of functions (that could grow when needed) that all work both on gpu and on cpu.

I personally find thememoryManager class a bit scary, and the different macros not so naturals to use. I have difficulties seeing the consequences of such a class as it is operating at a very low level. I think it could be worth introducing this class in a way that we could backtrack in the future, or replace by something else.

I think that thePABilinearForm is going in the right direction but a lot still needs to be done if we want to be able to propose the different levels of assembly: matrix free, partial assembly, local matrices, global matrix. This could have a non negligible impact on the rest of the design though.

Congratulations@camierjs , impressive work done in such a small time frame!

@jakubcerveny
Copy link
Member

I'm having trouble running ex1 with -p or -g with any other order than 1 and in 3D, so it's hard to look at performance. Is this a known limitation or is it just something that I'm doing wrong or something that can be fixed?

@camierjs
Copy link
MemberAuthor

Yes Jakub, it is a known limitation: I forgot to re-enable the kernels.
I'll do this as soon as I can.

@v-dobrev
Copy link
Member

Building on a Mac fails with the following errors:

general/memmng.cpp:319:24: error: member reference type 'struct __darwin_mcontext64 *' is a pointer; did you mean to use '->'?   context->uc_mcontext.gregs[REG_RIP]++;   ~~~~~~~~~~~~~~~~~~~~^                       ->general/memmng.cpp:319:25: error: no member named 'gregs' in '__darwin_mcontext64'   context->uc_mcontext.gregs[REG_RIP]++;   ~~~~~~~~~~~~~~~~~~~~ ^general/memmng.cpp:319:31: error: use of undeclared identifier 'REG_RIP'   context->uc_mcontext.gregs[REG_RIP]++;                              ^

v-dobrevand others added12 commitsApril 10, 2019 21:34
Small updates in ex1, ex1p, ex6, ex6p.
Lambda const propagation [lambda-capture-problem]
Updates in the examples, class BilinearForm and related [okina-bilinearform-updates]
@camierjscamierjs merged commitff9819e intomasterApr 12, 2019
@camierjscamierjs deleted the okina branchApril 12, 2019 01:45
@dmed256
Copy link
Member

🎉

@artv3
Copy link
Contributor

👍

@tzanio
Copy link
Member

Thank you@camierjs,@dmed256,@v-dobrev,@jakubcerveny,@jdahm,@pazner and@YohannDudouit for your contributions to this branch! 🚀

I know it was not easy, and I know that we are not done, but I think this is a really important step which would not have been possible without the hard and persistent work ofDavid,Veselin andJean-Sylvain. Thank you guys, you are truly amazing!

mlstowell, jakubcerveny, and acfisher reacted with hooray emoji

jonwong12 pushed a commit that referenced this pull requestMay 13, 2019
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@YohannDudouitYohannDudouitYohannDudouit approved these changes

@jdahmjdahmjdahm approved these changes

@tzaniotzaniotzanio approved these changes

@jakubcervenyjakubcervenyjakubcerveny approved these changes

@dmed256dmed256dmed256 approved these changes

@artv3artv3artv3 approved these changes

@paznerpaznerpazner approved these changes

@v-dobrevv-dobrevv-dobrev approved these changes

@acfisheracfisheracfisher approved these changes

@vladotomovvladotomovvladotomov approved these changes

@jamiebramwelljamiebramwelljamiebramwell approved these changes

Assignees

@camierjscamierjs

Projects
None yet
Milestone
mfem-4.0
Development

Successfully merging this pull request may close these issues.

12 participants
@camierjs@jakubcerveny@v-dobrev@artv3@tzanio@YohannDudouit@acfisher@pazner@jdahm@dmed256@vladotomov@jamiebramwell

[8]ページ先頭

©2009-2025 Movatter.jp