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

remove templates from make_sparsity_pattern #280

Merged
merged 1 commit into from
Nov 23, 2022

Conversation

necioglu
Copy link
Contributor

@necioglu necioglu commented Nov 23, 2022

Following PR of dealii dealii/dealii#14433 removes the templates from

dealii::MGTools::make_sparsity_pattern<dim, dim, dealii::DynamicSparsityPattern, Number>(
.

Made Exadg compatible with the changes. Thanks to @bergbauer

Closes #279

@peterrum
Copy link
Member

@nfehn Do we care about keeping compatibility with release 9.4?

@kronbichler
Copy link
Contributor

@nfehn Do we care about keeping compatibility with release 9.4?

Shouldn't we still be compatible? I don't see immediately why we would have needed to specify the template arguments and not rely on the automatic deduction for the 9.4 release.

@nfehn
Copy link
Member

nfehn commented Nov 23, 2022

I think we decided to only test against dealii:master, right?

@peterrum
Copy link
Member

I think we decided to only test against dealii:master, right?

No, we have not. We decided on dropping the testing of 9.3 a few weeks before the 9.4 release. I don't mind if we keep it like this. But there will be a time when we need to start versioning once we have more users.

@nfehn
Copy link
Member

nfehn commented Nov 23, 2022

But everything is green, so where would such an incompatibility with dealii-9.4 show up?

Principally, we should for now decide for a solution that allows most efficient progress. dealii promises to not make incompatible changes, but in practice it happens every few weeks to months. So I don't know what's best.

@nfehn nfehn added the bugfix Pull request fixing a bug label Nov 23, 2022
@nfehn nfehn merged commit 70c53d6 into exadg:master Nov 23, 2022
@kronbichler
Copy link
Contributor

dealii promises to not make incompatible changes, but in practice it happens every few weeks to months.

While I agree that we should (a) make sure to enable quick progress, but also (b) at some point have the facilities to support multiple versions of deal.II, I do not understand the issue here. Has someone tested that the current code actually is not compatible with version 9.4? In my experience, 80% of the issues we have with deal.II are things where a change we make is still compatible with the old version in deal.II (like I suppose is the case here, but I didn't test it). This helps us to make the code better overall.

@kronbichler kronbichler mentioned this pull request Nov 23, 2022
@nfehn
Copy link
Member

nfehn commented Nov 23, 2022

I just wanted to mention that this type of changes in ExaDG happen regularily. I do not criticize this. So I definitely appreciate overall progress in deal.II, but at the same time we have to find a good solution for ExaDG, and for that I think we need to take into account that such incompatible changes happen in deal.II.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix Pull request fixing a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ExaDG does not seem to work with current dealii master
4 participants