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

[Core] Adding kernel library list #9317

Merged
merged 21 commits into from
Dec 17, 2024
Merged

Conversation

loumalouomega
Copy link
Member

📝 Description
In order to skip tests depending on library disponibility

🆕 Changelog

  • Adding kernel library list
  • Expose to python
  • Added test
  • Reactivate CadModeler if triangle is available

@loumalouomega loumalouomega added Kratos Core Testing External library FastPR This Pr is simple and / or has been already tested and the revision should be fast labels Nov 10, 2021
@loumalouomega loumalouomega requested a review from a team as a code owner November 10, 2021 11:32
loumalouomega and others added 2 commits November 10, 2021 13:21
Co-authored-by: Philipp Bucher <[email protected]>
@pooyan-dadvand
Copy link
Member

@KratosMultiphysics/technical-committee we believe that the idea is very good and useful. Nevertheless, we see that having them all in the kernel is not the best way to maintain them.

We think that a better mechanism would be to use the new Registry under "third_party_libraries" and add each of them in their relative wrappers. (for example, json in our Parameters class)

@loumalouomega
Copy link
Member Author

@KratosMultiphysics/technical-committee we believe that the idea is very good and useful. Nevertheless, we see that having them all in the kernel is not the best way to maintain them.

I did it in order to be consistent with the applications

We think that a better mechanism would be to use the new Registry under "third_party_libraries" and add each of them in their relative wrappers. (for example, json in our Parameters class)

1- Is the Registry already merged?
2- I think not all libraries have a wrapper, F.E triangle is called in many places. The current structure using Registry should work fine

@loumalouomega
Copy link
Member Author

What is the status of this¿? https://github.com/orgs/KratosMultiphysics/teams/technical-committee

@loumalouomega loumalouomega removed the FastPR This Pr is simple and / or has been already tested and the revision should be fast label Sep 19, 2022
@loumalouomega
Copy link
Member Author

What is the status of this¿? github.com/orgs/KratosMultiphysics/teams/technical-committee

👋

@loumalouomega
Copy link
Member Author

I still think this is interesting

@loumalouomega
Copy link
Member Author

I still think this is interesting

@KratosMultiphysics/technical-committee

@loumalouomega
Copy link
Member Author

I still think this is interesting

@KratosMultiphysics/technical-committee

👋

@roigcarlo
Copy link
Member

Sorry we forgot to answer.
1 - Yes the registry is in place and usable.
2 - Answering for myself ( not TC ): For the ones that are used everywhere I suppose there is no other option than leave them there...

@loumalouomega
Copy link
Member Author

Sorry we forgot to answer. 1 - Yes the registry is in place and usable. 2 - Answering for myself ( not TC ): For the ones that are used everywhere I suppose there is no other option than leave them there...

But here no registry is used...

@roigcarlo
Copy link
Member

Yep, that why we asked you to use the registry 😄

@loumalouomega
Copy link
Member Author

Yep, that why we asked you to use the registry 😄

But it is a list, I am not using classes. What's the point?

@roigcarlo
Copy link
Member

That it becomes available everywhere without depending on having a reference to the kernel. Which btw makes sense because that info is not related to the kernel.

@loumalouomega
Copy link
Member Author

That it becomes available everywhere without depending on having a reference to the kernel. Which btw makes sense because that info is not related to the kernel.

Okay, in that case I would need to study this

@loumalouomega
Copy link
Member Author

That it becomes available everywhere without depending on having a reference to the kernel. Which btw makes sense because that info is not related to the kernel.

Okay, in that case I would need to study this

Ups, it looks a bit of work... and create a new class

@RiccardoRossi
Copy link
Member

this one is delegated to @roigcarlo, who is requesting to use the registry as a tool for querying for available libraries

@loumalouomega
Copy link
Member Author

this one is delegated to @roigcarlo, who is requesting to use the registry as a tool for querying for available libraries

Okay

@loumalouomega
Copy link
Member Author

(just merging master, this branch wasn't merged from long time ago, I will take a look at @roigcarlo suggestion later)

@loumalouomega loumalouomega requested a review from a team as a code owner December 16, 2024 20:41
@loumalouomega
Copy link
Member Author

SHould be fine now @roigcarlo

@loumalouomega
Copy link
Member Author

BTW, there is any reason why this is empty?

static std::unordered_set<std::string> application_list;

@loumalouomega loumalouomega merged commit 046ab0d into master Dec 17, 2024
11 checks passed
@loumalouomega loumalouomega deleted the core/adding-kernel-library-list branch December 17, 2024 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

5 participants