Skip to content

Conversation

@javoha
Copy link
Member

@javoha javoha commented Jun 12, 2025

Description

New PR for the evaluating the computation time for custom grids. Test now includes model computation for pure model, after adding a topography and using the compute_model_at function with 1000 random points.
I removed the test that deep created to clean up.

Just fyi - here are the resulting times on my computer:

image

Added a warning message to compute_model_at function to alert users about potential side effects when setting a custom grid and computing the model. Modified the set_custom_grid function to include a reset parameter, which is now set to True in compute_model_at to ensure proper grid reset.

Removed code in the solutions setter that was setting solutions per group, as this was causing issues with the custom grid functionality.

Added comprehensive tests to measure and compare computation times across different grid configurations:

  • Dense grid computation (with consistency checks)
  • Topography grid vs dense grid computation
  • Custom grid computation with random coordinates

Checklist

  • My code uses type hinting for function and method arguments and return values.
  • I have created tests which cover my code.
  • The test code either 1. demonstrates at least one valuable use case (e.g. integration tests)
    or 2. verifies that outputs are as expected for given inputs (e.g. unit tests).
  • New tests pass locally with my changes.

Relates to: gempy-project/gempy_engine#16 (comment)

@graphite-app graphite-app bot added the gempy 3 Will come with the next major update label Jun 12, 2025
@graphite-app
Copy link

graphite-app bot commented Jun 12, 2025

Graphite Automations

"Add gempy label" took an action on this PR • (06/12/25)

1 label was added to this PR based on Miguel de la Varga's automation.

Copy link
Member

Leguark commented Jun 12, 2025

So yeah two small bugs but that it was messing up computations times:

  1. when we were computing_at, we were creating a custom grid but we were not deactivating the previous grids. Now I do deactivate everything except the custom grid and print a warning.

  2. This one was harder to pin. When we where computing dual contouring we were activating compute scalar gradient and left it on in memory. In practice what was doing is that a second compute_model was taking much longer because we were actually computing scalarfield gradients all the way.

These two things are mostly patches for gempy_engine. I will do a pre-release as soon as the CI ends

@javoha
Copy link
Member Author

javoha commented Jun 12, 2025

So yeah two small bugs but that it was messing up computations times:

  1. when we were computing_at, we were creating a custom grid but we were not deactivating the previous grids. Now I do deactivate everything except the custom grid and print a warning.
  2. This one was harder to pin. When we where computing dual contouring we were activating compute scalar gradient and left it on in memory. In practice what was doing is that a second compute_model was taking much longer because we were actually computing scalarfield gradients all the way.

These two things are mostly patches for gempy_engine. I will do a pre-release as soon as the CI ends

That's great to hear, thanks a lot for looking into it!

Modified the approved test file to set `compute_scalar_gradient` to `false`, ensuring consistency with test expectations and configuration.
@Leguark Leguark merged commit c010735 into main Jun 13, 2025
2 checks passed
@Leguark Leguark deleted the compute_at_compuation_time branch June 13, 2025 08:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gempy 3 Will come with the next major update model computation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants