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.

Sign up for GitHub

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

Jump to bottom

Use OrderedSet for cellsets #631

Closed
fredrikekre opened this issue Mar 22, 2023 · 5 comments · Fixed by #834
Closed

Use OrderedSet for cellsets #631

fredrikekre opened this issue Mar 22, 2023 · 5 comments · Fixed by #834
Labels
good first issue Something to work on as a new contributor! refactoring Code refactoring, no functional change.
Milestone
v1.0.0

Comments

@fredrikekre
Copy link
Member

I think we should use OrderedSets from https://github.com/JuliaCollections/OrderedCollections.jl for, at least, the cellsets in the grid. This will ensure that we have both fast lookup and that we can iterate cells in order. This will require JuliaCollections/OrderedCollections.jl#100 and a new release of that package so that we can sort! in the Grid constructor and in addcellset.

xref discussion in #625
@termi-official termi-official added the refactoring Code refactoring, no functional change. label Mar 22, 2023
@termi-official
Copy link
Member

Should I address this already in the linked PR or leave it as a good first issue?

@fredrikekre
Copy link
Member Author

This could probably be pretty self-contained PR (easier to review). Opened this mostly to see if people liked the idea :)

@termi-official termi-official added the good first issue Something to work on as a new contributor! label Mar 22, 2023
@fredrikekre fredrikekre added this to the 0.4.0 milestone Mar 23, 2023
@kimauth kimauth mentioned this issue Mar 24, 2023
Closed
@lijas
Copy link
Collaborator

lijas commented Mar 25, 2023

What's the drawback between ordered sets and normal sets? :)

@koehlerson
Copy link
Member

What's the drawback between ordered sets and normal sets? :)

If the problem is big and you iterate over the set, i.e. unordered, you need to visit very different chunks of data for e.g. internal variables

@termi-official
Copy link
Member

The bigger issue is that "normal" sets are unordered and ordering may even mutate. E.g. you cannot easily iterate over the entries in ascending order.

@termi-official termi-official mentioned this issue Nov 1, 2023
Merged
@KnutAM KnutAM mentioned this issue Feb 16, 2024
Closed
2 tasks
@termi-official termi-official mentioned this issue May 13, 2024
Open
fredrikekre added a commit that referenced this issue May 19, 2024
@fredrikekre @termi-official
Replace Set with OrderedSets in grid collections
bee3d7b 
This patch changes the use of `Set` to `OrderedSet` in the grid. This
means that e.g. loops over these sets follow the original specified
order. This is important for performance since it can reduce
cache-misses, for example. Fixes #631.

Co-authored-by: Fredrik Ekre <ekrefredrik@gmail.com>
Co-authored-by: Dennis Ogiermann <termi-official@users.noreply.github.com>
fredrikekre added a commit that referenced this issue May 19, 2024
@fredrikekre @termi-official
Replace Set with OrderedSets in grid collections
fcb5dee 
This patch changes the use of `Set` to `OrderedSet` in the grid. This
means that e.g. loops over these sets follow the original specified
order. This is important for performance since it can reduce
cache-misses, for example. Fixes #631. Closes #654.

Co-authored-by: Fredrik Ekre <ekrefredrik@gmail.com>
Co-authored-by: Dennis Ogiermann <termi-official@users.noreply.github.com>
fredrikekre added a commit that referenced this issue May 19, 2024
@fredrikekre @termi-official
Replace Set with OrderedSets in grid collections
de407e7 
This patch changes the use of `Set` to `OrderedSet` in the grid. This
means that e.g. loops over these sets follow the original specified
order. This is important for performance since it can reduce
cache-misses, for example. Fixes #631. Closes #654.

Co-authored-by: Fredrik Ekre <ekrefredrik@gmail.com>
Co-authored-by: Dennis Ogiermann <termi-official@users.noreply.github.com>
fredrikekre added a commit that referenced this issue May 20, 2024
@fredrikekre @termi-official
Replace Set with OrderedSets in grid collections
4634fde 
This patch changes the use of `Set` to `OrderedSet` in the grid. This
means that e.g. loops over these sets follow the original specified
order. This is important for performance since it can reduce
cache-misses, for example. Fixes #631. Closes #654.

Co-authored-by: Fredrik Ekre <ekrefredrik@gmail.com>
Co-authored-by: Dennis Ogiermann <termi-official@users.noreply.github.com>
fredrikekre added a commit that referenced this issue May 20, 2024
@fredrikekre @termi-official
Replace Set with OrderedSets in grid collections
ec01184 
This patch changes the use of `Set` to `OrderedSet` in the grid. This
means that e.g. loops over these sets follow the original specified
order. This is important for performance since it can reduce
cache-misses, for example. Fixes #631. Closes #654.

Co-authored-by: Fredrik Ekre <ekrefredrik@gmail.com>
Co-authored-by: Dennis Ogiermann <termi-official@users.noreply.github.com>
fredrikekre added a commit that referenced this issue May 20, 2024
@fredrikekre @termi-official
Replace Set with OrderedSets in grid collections
bcae4a4 
This patch changes the use of `Set` to `OrderedSet` in the grid. This
means that e.g. loops over these sets follow the original specified
order. This is important for performance since it can reduce
cache-misses, for example. Fixes #631. Closes #654.

Co-authored-by: Fredrik Ekre <ekrefredrik@gmail.com>
Co-authored-by: Dennis Ogiermann <termi-official@users.noreply.github.com>
fredrikekre added a commit that referenced this issue May 20, 2024
@termi-official @fredrikekre
Replace Set with OrderedSets in grid collections (#834)
15db94b 
This patch changes the use of `Set` to `OrderedSet` in the grid. This
means that e.g. loops over these sets follow the original specified
order. This is important for performance since it can reduce
cache-misses, for example. Fixes #631. Closes #654.

Co-authored-by: Fredrik Ekre <ekrefredrik@gmail.com>
Co-authored-by: Dennis Ogiermann <termi-official@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first issue Something to work on as a new contributor! refactoring Code refactoring, no functional change.
Projects
None yet
Milestone
v1.0.0
Development

Successfully merging a pull request may close this issue.

Introduce OrderedSets Ferrite-FEM/Ferrite.jl
4 participants
@lijas @termi-official @fredrikekre @koehlerson