feat(problem)!: add a verbose variant of simulate()#252
Open
g-braeunlich wants to merge 2 commits into
Open
Conversation
5f6f4e7 to
983707e
Compare
markfuge
reviewed
May 16, 2026
Member
markfuge
left a comment
There was a problem hiding this comment.
@g-braeunlich Overall, good first pass. A few questions for me that came up when reviewing:
- For SimulationResult -- how does the user know, e.g., what the elements of the SimulationResult are? For example, in the multi-objective case in ThermoElastic, how does the user know that in the NDArray for
objective_valuesthe first one is structural compliance, the second is thermal compliance, etc.? This was the case before this PR as well, but it just occurred to me now. This would also occur if we started to add stuff to SimulationResult (i.e., how does the user know what the other fields are and how to call them and what they contain? - For the docs -- where are these changes documented in the docs? The API page still only mentions
simulateand notsimulate_verbosefor example. - Related to the documentation, what should the problem authors add to their documentation pages about the
simulate_verboseoutputs such that this is discernable/useable by users, and not just buried in the code?
Other than this, things look great!
BREAKING CHANGE: The `Problem` class now has `simulate_verbose()` as a new required method which has to be implemented by custom `Problem` classes.
983707e to
606116e
Compare
Collaborator
Author
|
@markfuge Thanks for the review. Forgot to link the new method doc to the API. This should be done now. Also added some additional notes to address 3. @arrayclass
class Arr(ArrayClassBase[np.float64]):
x: Annotated[float, Index[0]]
v: Annotated[float, Index[1:]]
a = Arr(np.array([1.0, 2.0, 3.0]))
a.v = [-2.0, -3.0]
np.testing.assert_equal(asarray(a), np.array([1.0, -2.0, -3.0]))I.e. the underlying data would still be a numpy array, but its elemets could be accessed by identifiers. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
BREAKING CHANGE: The
Problemclass now hassimulate_verbose()as a new required method which has to be implemented by customProblemclasses.Description
Currently,
Problem.simulate()returns a numpy array containing the objective values measuring the performance of the simulated design.This PR Introduces a new
Problem.simulate_verbose()method which returns aSimulationResult(dataclass) object which contains a fieldobjective_values- the return value of the currentsimulate()method. Specific problem classes can now derive fromSimulationResultand add more data.Problem.simulate()has now a default implementation which callsProblem.simulate_verbose()and returnssimulation_result.objective_values. Therefore at least the methodsimulate()does not receive a breaking change.Fixes #251
Type of change
Please delete options that are not relevant.
Checklist:
pre-commitchecks withpre-commit run --all-filesruff check .andruff formatmypy .Reviewer Checklist: