Skip to content

Optimize extract#37

Merged
jourdain merged 8 commits intoKitware:masterfrom
danlipsa:optimize_extract
Apr 8, 2026
Merged

Optimize extract#37
jourdain merged 8 commits intoKitware:masterfrom
danlipsa:optimize_extract

Conversation

@danlipsa
Copy link
Copy Markdown
Collaborator

@danlipsa danlipsa commented Apr 7, 2026

No description provided.

danlipsa added 2 commits April 2, 2026 14:28
Caching only the ghost cells won't work as RemoveGhostCells modifies
the output. Only new arrays are modified using PedigreeIds
@danlipsa
Copy link
Copy Markdown
Collaborator Author

danlipsa commented Apr 7, 2026

@jourdain Please review

@danlipsa
Copy link
Copy Markdown
Collaborator Author

danlipsa commented Apr 7, 2026

Note that I had to update the input for the mappers (in my script) on the second render, otherwise I was seeing a crash at the second render: https://gitlab.kitware.com/danlipsa/quickview_optimize/-/commit/b3cc77daf462693b27a25b7daf8bbd79891aba57

I see a similar crash in Quickview, and it seems we don't do that there - but maybe I don't follow the code.
To get that crash in Quickview:

  1. Load the 120 dataset (simulation + connectivity)
  2. Choose BURDEN1 and CLDLOW. Load 2 variables
  3. Lat/lon cropping. Move the left slider to some value, than move it back to 0.

@danlipsa
Copy link
Copy Markdown
Collaborator Author

danlipsa commented Apr 7, 2026

Seems there is an additional issue going on. I still get the crash if I update only the "trim". If I update both the trim and projection I don't get the crash (in my script).

@danlipsa
Copy link
Copy Markdown
Collaborator Author

danlipsa commented Apr 8, 2026

Note that I had to update the input for the mappers (in my script) on the second render, otherwise I was seeing a crash at the second render: https://gitlab.kitware.com/danlipsa/quickview_optimize/-/commit/b3cc77daf462693b27a25b7daf8bbd79891aba57

I see a similar crash in Quickview, and it seems we don't do that there - but maybe I don't follow the code. To get that crash in Quickview:

  1. Load the 120 dataset (simulation + connectivity)
  2. Choose BURDEN1 and CLDLOW. Load 2 variables
  3. Lat/lon cropping. Move the left slider to some value, than move it back to 0.

This is fixed.

@danlipsa
Copy link
Copy Markdown
Collaborator Author

danlipsa commented Apr 8, 2026

@jourdain Please review. This fixes all issues I've been able to find in quickview.

@danlipsa
Copy link
Copy Markdown
Collaborator Author

danlipsa commented Apr 8, 2026

Seems there is an additional issue going on. I still get the crash if I update only the "trim". If I update both the trim and projection I don't get the crash (in my script).

This is fixed

@jourdain
Copy link
Copy Markdown
Collaborator

jourdain commented Apr 8, 2026

LGTM, should I merge?

@jourdain jourdain self-requested a review April 8, 2026 15:38
@danlipsa
Copy link
Copy Markdown
Collaborator Author

danlipsa commented Apr 8, 2026

@jourdain Two more commits. First one didn't reproduced it, but I think it better than before. The second one - a DeepCopy that was bothering me so I debugged to see what was the cause.

@danlipsa
Copy link
Copy Markdown
Collaborator Author

danlipsa commented Apr 8, 2026

I think now it is ready to be merged. You want to take another quick look?

@jourdain jourdain merged commit 6653657 into Kitware:master Apr 8, 2026
1 check passed
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