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

Easier generation of description dictionaries #367

Merged
merged 3 commits into from
Oct 26, 2023

Conversation

brownbaerchen
Copy link
Contributor

@tlunet rightfully remarked that the setup of a problem to run is not very inviting to new users. I have to admit, after 2 years of using pySDC, I still copy paste a description every time I need a new one. This PR is meant as a starting point to address this. It introduces a new function called generate_description and you can basically dump all variables in there and it will distribute them to the correct sub dictionaries for step_params and so on.
Also, there is a new interface for a default sweeper for a given problem. Most of the time, there is an obvious choice for a certain sweeper to use, but this needs to be implemented for each problem individually.

@codecov
Copy link

codecov bot commented Oct 24, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (f5658fc) 73.26% compared to head (6893cc2) 73.29%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #367      +/-   ##
==========================================
+ Coverage   73.26%   73.29%   +0.02%     
==========================================
  Files         270      271       +1     
  Lines       23097    23122      +25     
==========================================
+ Hits        16922    16947      +25     
  Misses       6175     6175              
Files Coverage Δ
pySDC/core/Level.py 100.00% <100.00%> (ø)
pySDC/core/Problem.py 100.00% <100.00%> (ø)
pySDC/helpers/setup_helper.py 100.00% <100.00%> (ø)
...C/implementations/problem_classes/generic_ND_FD.py 94.80% <100.00%> (+0.28%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

def get_default_sweeper_class(cls):
from pySDC.implementations.sweeper_classes.generic_implicit import generic_implicit

return generic_implicit
Copy link
Member

Choose a reason for hiding this comment

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

Why is that the default sweeper class for this problem type? Could also be IMEX?

Copy link
Member

Choose a reason for hiding this comment

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

Indeed, but I guess the IMEX as standard should come with a bigger rework of pySDC, right ?

@pancetta pancetta merged commit 70cbcbb into Parallel-in-Time:master Oct 26, 2023
25 checks passed
@brownbaerchen
Copy link
Contributor Author

I actually planned to address your concerns a bit. The idea behind the sweeper is that for a given choice of dtype_f, there are only a few sweepers that are compatible at all and there is usually one that makes sense as a default. Here, for instance, the dtype_f is mesh, so the IMEX sweeper is not compatible. If you inherit from this and change dtype_f, you would have to overload the function for the default sweeper. I think this is sensible enough, but requires some modification of all problem classes. You may want the MPI version of the generic_implicit sweeper here, but then you should supply this manually because it should not be the default.
I was thinking maybe a more elegant solution would be to access the class dtype_f attribute of the problem class and check what sweeper fits this. What do you think about this? Since I only ever worked with the mesh kind of datatypes, I don't know if this would work for Fenics stuff as well. I would need a list of sensible default sweepers for every datatype.
Maybe this list could include defaults for implicit and explicit preconditioners as well.

@pancetta
Copy link
Member

I do like the idea of "allowing" only certain combinations of data- and sweeper-type, or to prevent wrong combinations. I also like the idea of doing this automatically somehow, but we really need to make sure we don't add more obstacles to the code, esp. ones we're going to regret at some point. And esp. not because there is no strong need to.

@brownbaerchen
Copy link
Contributor Author

I guess we already have that. If you use a sweeper with a datatype that are incompatible, it will raise an error in update_nodes or earlier. Maybe it will not give a helpful error statement.

What I want to do here is just to speed up time-to-simulation for inexperienced users. What I want is the user to be able to supply a problem class and dump parameters and get a description back that is ready to run sth.
But this does not replace the current thing of just setting up all the dictionaries yourself. It is just an alternative. In this way, I think it's not an obstacle.

@pancetta
Copy link
Member

I get what you're saying. If the specification of sweepers is a sensible one, can be overruled and "users" know about this, then I'm on board.

@brownbaerchen
Copy link
Contributor Author

Cool, then I need a list of what sweeper to use with which datatype.

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