Skip to content

Some additions to derived adjacencies#89

Merged
chmerdon merged 5 commits intomasterfrom
chmerdon/extension_derived
May 13, 2025
Merged

Some additions to derived adjacencies#89
chmerdon merged 5 commits intomasterfrom
chmerdon/extension_derived

Conversation

@chmerdon
Copy link
Copy Markdown
Member

@chmerdon chmerdon commented May 5, 2025

  • possibility to store transposed adjacencies NodeCells, NodeFaces and NodeEdges
  • docstrings for all derived adjacencies
  • EdgeNodes now also instantiates parental adjacencies if it has a ParentGrid (like FaceNodes did already, implementing issue implement EdgeParents in instatiation of EdgeNodes #88)

…es, NodeEdges

- dcostrings for all derived types
@pjaap
Copy link
Copy Markdown
Member

pjaap commented May 5, 2025

Nice. Can you add a Changelog entry and bump the minor version number? I think we can ignore the nightly failure.

@chmerdon
Copy link
Copy Markdown
Member Author

chmerdon commented May 5, 2025

ok,

another question: in FaceNodes instantiation also atranspose(xgrid[CellNodes]) is needed, should this be switched to grid[NodeCells] as well (triggering that this is saved always in the grid as soon as FaceNodes are involved)?

@chmerdon chmerdon requested a review from j-fu May 5, 2025 12:08
@pjaap
Copy link
Copy Markdown
Member

pjaap commented May 5, 2025

Yes, sounds always reasonable to keep the result atranspose if we compute it.

@chmerdon
Copy link
Copy Markdown
Member Author

chmerdon commented May 5, 2025

Yes, sounds always reasonable to keep the result atranspose if we compute it.

Usually this step is a very fast and non-time-critical step in the other global adjacency instantiations where this is used. So not saving the same numbers twice might be also a reasonable default behavior.

@pjaap
Copy link
Copy Markdown
Member

pjaap commented May 5, 2025

True. I tend to keeps thing as they are if we are not sure. Maybe add a comment at the specific code line?

chmerdon added 2 commits May 5, 2025 14:55
…, lines with atranpose have a comment indicating that one might use the new storage-inducing adjacency types
@pjaap
Copy link
Copy Markdown
Member

pjaap commented May 5, 2025

When adding this branch I get the warning

┌ ExtendableGrids
│  ┌ Warning: Replacing docs for `ExtendableGrids.BEdgeRegions :: Union{}` in module `ExtendableGrids`
│  └ @ Base.Docs docs/Docs.jl:243
└  

@chmerdon chmerdon removed the request for review from j-fu May 12, 2025 14:54
@chmerdon chmerdon force-pushed the chmerdon/extension_derived branch from ef9b472 to db5d9bc Compare May 12, 2025 14:57
@chmerdon chmerdon requested a review from j-fu May 12, 2025 15:00
Copy link
Copy Markdown
Member

@j-fu j-fu left a comment

Choose a reason for hiding this comment

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

Looks good to me.

We should notice the issue of "garbage collection" of grid items.
In the moment I have no idea how to proceed with this.

@chmerdon
Copy link
Copy Markdown
Member Author

I opened issue #90 on garbage collection and will merge this PR now.

@chmerdon chmerdon merged commit d3555c3 into master May 13, 2025
19 of 27 checks passed
@pjaap pjaap deleted the chmerdon/extension_derived branch May 14, 2025 07:59
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