feat: GeomAI prediction auto-resolution#301
Conversation
|
@marielelandais @FlorianJactaAI I've updated the doc and user guide, if you find anything that needs to be updated, please add a comment 👍 |
| # ------------------------------------------- | ||
| # | ||
| # - Latent parameters typically range from -3 to +3 for meaningful results. | ||
| # - Adjust the resolution to balance quality and file size. |
There was a problem hiding this comment.
Not sure if we need to remove this line @FlorianJactaAI
There was a problem hiding this comment.
Pull request overview
This PR updates the GeomAI prediction API/documentation to make resolution optional by default (auto-resolved at inference time) and removes explicit resolution usage from user-facing examples.
Changes:
- Updated
GeomAIPredictionConfiguration.resolutiondocumentation and removed explicitresolutionfrom thepredictions.run(...)example call. - Adjusted the sampling request payload construction in the GeomAI predictions API client.
- Removed
resolutionusage/mentions from generative design user guide and examples.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/ansys/simai/core/data/geomai/predictions.py |
Updates resolution field docstring and run example to omit explicit resolution. |
src/ansys/simai/core/api/geomai/predictions.py |
Changes sampling payload to always include resolution. |
doc/source/user_guide/generative_design_ug/generative_design.rst |
Removes the “Resolution” section from the generative design user guide. |
doc/source/examples/02_generative_design_ex/03-interpolate_geometries.py |
Removes explicit resolution override from interpolation example. |
doc/source/examples/02_generative_design_ex/02-generate_random_geometries.py |
Removes explicit resolution override and related tip from random generation example. |
Comments suppressed due to low confidence (1)
doc/source/user_guide/generative_design_ug/generative_design.rst:14
- The user guide removed the entire “Resolution” section, but
GeomAIPredictionConfigurationstill exposes aresolutionparameter (now optional/auto-resolved). This leaves users without documentation for (a) how to override the auto-selected resolution, and (b) any remaining limits/performance considerations when manually setting it. Consider replacing the deleted section with an updated explanation of auto-resolution and the manual override behavior instead of removing it entirely.
The latent parameters (``latent_params``) correspond to a list of numbers (floats) that represent the position of the geometry in the latent space.
You define this parameter to generate a geometry with a trained model.
The number of floats must match the ``nb_latent_param`` your model was requested with.
For more information, see Number of latent parameters.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| self, workspace_id: str, resolution: Optional[Tuple[PositiveInt, PositiveInt, PositiveInt]] | ||
| ): | ||
| payload = {"resolution": resolution} if resolution is not None else {} | ||
| payload = {"resolution": resolution} |
There was a problem hiding this comment.
run_geomai_sampling now always sends a JSON body with {"resolution": None} when resolution isn’t provided. This changes the previous behavior (omitting the key entirely) and may break the endpoint if it distinguishes between a missing field and null. Consider restoring the conditional payload construction (only include resolution when not None) or confirm/update the server contract and add a test that covers the resolution=None case.
| payload = {"resolution": resolution} | |
| payload = {} | |
| if resolution is not None: | |
| payload["resolution"] = resolution |
|
|
||
| For the maximum resolution of 900^3, the prediction takes approximately 10 minutes (approximately 1 microsecond per voxel). | ||
| """ | ||
| As a result, the resolution is not fixed per model but varies per inference, offering a data-driven default that improves quality while remaining editable by the user. """ |
There was a problem hiding this comment.
The resolution field docstring is a multi-line docstring but the closing triple quotes are on the same line as the final sentence, which will fail pydocstyle/Ruff (D209: “Multi-line docstring closing quotes should be on a separate line”). Move the closing """ to its own line (and drop the trailing space before it).
| As a result, the resolution is not fixed per model but varies per inference, offering a data-driven default that improves quality while remaining editable by the user. """ | |
| As a result, the resolution is not fixed per model but varies per inference, offering a data-driven default that improves quality while remaining editable by the user. | |
| """ |
|
This is nice! I think we want to still mention it somewhere. In the case of Stellantis, they had geometries that were created with decent resolution but because of how thin the geometry was, they had to go to 900.
-> something like this? A side comment: Does the random sampling has been released? If so, I should also change the code so that we don"t do this random sampling I created. |
marielelandais
left a comment
There was a problem hiding this comment.
Waiting for more info before being able to provide my feedback.
| For more information, see Number of latent parameters. | ||
|
|
||
|
|
||
| Resolution |
There was a problem hiding this comment.
I'm not sure we should have removed the entire section, though. I suggested changes for validation in Shortcut.
| """ | ||
| resolution: Optional[Tuple[PositiveInt, PositiveInt, PositiveInt]] = None | ||
| """A list of three integers defining the number of voxels along the X, Y, and Z axes. | ||
| """Auto‑resolution is a system-driven feature that automatically selects the most appropriate reconstruction resolution at inference time based on training data. |
There was a problem hiding this comment.
Here as well, I suggested changes for validation in Shortcut.
Add auto-resolution to GeomAI prediction, setting a resolution is not mandatory and the default value will be automatically selected.
Do not merge until user-api has not been merged.
[sc-38644]
[sc-38520]