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

WIP: Traits for ScalarValues / VectorValues #404

Merged
merged 5 commits into from
Jan 18, 2022
Merged

WIP: Traits for ScalarValues / VectorValues #404

merged 5 commits into from
Jan 18, 2022

Conversation

kimauth
Copy link
Member

@kimauth kimauth commented Jan 4, 2022

A suggestion on using traits instead of Unions for defining ScalarValues and VectorValues.

I want to use it for subtyping Values without copying a bunch of code from Ferrite. Could also be useful for custom definition of (less common?) problems like the one mentioned in #398.

I'm happy to hear some feedback, if this is appreciated I'll add some docs as well.

Copy link
Collaborator

@lijas lijas left a comment

Choose a reason for hiding this comment

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

I think it looks good, and can be merged...

But we can use the traits in the reinit! methods aswell now:

isa(cv, CellVectorValues) && (n_func_basefuncs *= dim)

src/FEValues/common_values.jl Outdated Show resolved Hide resolved
src/FEValues/common_values.jl Outdated Show resolved Hide resolved
@lijas
Copy link
Collaborator

lijas commented Jan 18, 2022

Just a general question... why not use VectorValues as supertype instead of Face/CellValues. What dictates what trait/property should be used as supertype, and what should be implemented as traits?

@kimauth
Copy link
Member Author

kimauth commented Jan 18, 2022

The reason here was to literally replace the old scalar/vector value unions. Sure could be the other way around, but that would be a larger redesign. Would you argue that it really should be the other way around?

@lijas
Copy link
Collaborator

lijas commented Jan 18, 2022

Not worth redesigning I think, I was just wondering if it would be better and if there is some helpful rule to decide what should be used as traits/supertypes

@kimauth kimauth merged commit d1503b7 into master Jan 18, 2022
@kimauth kimauth deleted the ka/traits branch January 18, 2022 13:34
koehlerson pushed a commit that referenced this pull request Apr 22, 2022
* traits for ScalarValues & VectorValues

* remove outdated / wrong comments

* rename to ScalarValue/VectorValued

* replace isa(VectorValues)

* change version to 0.3.2
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.

5 participants