fix(thermoelastic2d): align column names with v1 dataset#247
fix(thermoelastic2d): align column names with v1 dataset#247
Conversation
Rename `volfrac` → `volume_fraction_target` in Conditions and fea_model, and `volume_fraction` → `volume_fraction_error` in objectives and return dicts to match the thermoelastic_2d_v1 HuggingFace dataset schema. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Aligns thermoelastic2d v0 problem input/output field names with the v1 HuggingFace dataset schema, so dataset-driven configs and objective columns match simulator/optimizer expectations.
Changes:
- Renamed condition key
volfrac→volume_fraction_targetinThermoElastic2D.ConditionsandFeaModel.run()boundary-condition parsing. - Renamed objective/result key
volume_fraction→volume_fraction_erroracross objectives and returned result dicts. - Updated the 2D FEA model’s example
bcsto use the new key name.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| engibench/problems/thermoelastic2d/v0.py | Updates objective name and condition field to match v1 dataset columns; updates simulate() to read the renamed result key. |
| engibench/problems/thermoelastic2d/model/fea_model.py | Updates expected BC key and returned result dict key to match v1 dataset columns; updates local __main__ example. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| results = FeaModel(plot=False, eval_only=True).run(boundary_dict, x_init=design) | ||
| return np.array([results["structural_compliance"], results["thermal_compliance"], results["volume_fraction"]]) | ||
| return np.array([results["structural_compliance"], results["thermal_compliance"], results["volume_fraction_error"]]) |
There was a problem hiding this comment.
This method returns an np.ndarray (objective values), but the docstring currently describes returning a dict of named objective values. Please update the docstring/return description to match the actual return type and clarify the ordering of objectives in the returned array.
| ) | ||
| """Binary NxN matrix specifying elements that have a heat sink""" | ||
| volfrac: Annotated[float, bounded(lower=0.0, upper=1.0).category(THEORY)] = 0.3 | ||
| volume_fraction_target: Annotated[float, bounded(lower=0.0, upper=1.0).category(THEORY)] = 0.3 |
There was a problem hiding this comment.
Renaming volfrac to volume_fraction_target is a breaking change for any callers passing configs with the old key. To avoid silent misconfiguration (the config merge only updates known keys), consider accepting volfrac as a deprecated alias (mapping it to volume_fraction_target) and/or raising a clear error when volfrac is provided.
| nely = fe_w - 1 | ||
|
|
||
| volfrac = bcs["volfrac"] | ||
| volfrac = bcs["volume_fraction_target"] | ||
| n = nely * nelx # Total number of elements |
There was a problem hiding this comment.
run() now requires bcs["volume_fraction_target"], which will raise KeyError for existing callers still providing volfrac. Consider supporting volfrac as a deprecated fallback (or at least raising a clearer ValueError with the expected key names) to make this migration less error-prone.
| Args: | ||
| bcs (dict[str, any]): A dictionary containing boundary conditions and problem parameters. | ||
| Expected keys include: | ||
| - 'volfrac' (float): Target volume fraction. | ||
| - 'volume_fraction_target' (float): Target volume fraction. | ||
| - 'fixed_elements' (np.ndarray): NxN binary array encoding the location of fixed elements. |
There was a problem hiding this comment.
The docstring was updated to list volume_fraction_target, but the rest of the Returns: section still refers to keys like 'sc', 'tc', and 'vf'. Please update the return-key documentation to match the actual dict keys returned (e.g., structural_compliance, thermal_compliance, volume_fraction_error).
| return { | ||
| "structural_compliance": f0valm, | ||
| "thermal_compliance": f0valt, | ||
| "volume_fraction": vf_error, | ||
| "volume_fraction_error": vf_error, | ||
| } |
There was a problem hiding this comment.
Renaming the returned key from volume_fraction to volume_fraction_error is a breaking change for any downstream consumers of FeaModel.run() results. If you need backwards compatibility, consider returning both keys for a deprecation window (or documenting this change prominently).
Summary
volfrac→volume_fraction_targetinConditionsdataclass andfea_model.pyto match the v1 HuggingFace dataset column namevolume_fraction→volume_fraction_errorin objectives tuple and allfea_model.pyreturn dicts to match the v1 datasetTest plan
conditions_keysall exist inthermoelastic_2d_v1dataset columnsobjectivesnames all exist inthermoelastic_2d_v1dataset columnssimulate()successfully on a random design from the datasetoptimize()successfully withmax_iter=5🤖 Generated with Claude Code