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

Deprecate assemble! with local vector before local matrix #1059

Merged
merged 2 commits into from
Sep 19, 2024
Merged

Conversation

fredrikekre
Copy link
Member

This patch deprecates the signature
assemble!(assembler, dofs, local_vector, local_matrix) in favor of assemble!(assembler, dofs, local_matrix, local_vector). The former signature was not used in many places, not supported by e.g. the block matrix assembler, and it is confusing that there are two options. See also #707 for a similar change to start_assemble.

In addition, if the assembler is used just for matrix assembly, this patch removes an unnecessary allocation of an empty vector for every call to assemble!.

@fredrikekre fredrikekre added this to the v1.0.0 milestone Sep 19, 2024
Copy link

codecov bot commented Sep 19, 2024

Codecov Report

Attention: Patch coverage is 66.66667% with 2 lines in your changes missing coverage. Please review.

Project coverage is 93.59%. Comparing base (5ec6c84) to head (0d19930).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
src/deprecations.jl 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1059      +/-   ##
==========================================
- Coverage   93.63%   93.59%   -0.04%     
==========================================
  Files          39       39              
  Lines        6013     6011       -2     
==========================================
- Hits         5630     5626       -4     
- Misses        383      385       +2     

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

Copy link
Member

@KnutAM KnutAM left a comment

Choose a reason for hiding this comment

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

Good change!

Needs changelog I guess, and I think my suggestion is required?

This patch deprecates the signature
`assemble!(assembler, dofs, local_vector, local_matrix)` in favor of
`assemble!(assembler, dofs, local_matrix, local_vector)`. The former
signature was not used in many places, not supported by e.g. the block
matrix assembler, and it is confusing that there are two options. See
also #707 for a similar change to `start_assemble`.

In addition, if the assembler is used just for matrix assembly, this
patch removes an unnecessary allocation of an empty vector for every
call to `assemble!`.
@fredrikekre fredrikekre force-pushed the fe/fe branch 2 times, most recently from 5136d1b to 21c96a8 Compare September 19, 2024 18:14
@fredrikekre fredrikekre merged commit 0fe2ed8 into master Sep 19, 2024
9 of 11 checks passed
@fredrikekre fredrikekre deleted the fe/fe branch September 19, 2024 23:22
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.

2 participants