Skip to content

fix(thermoelastic2d): align column names with v1 dataset #247

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mkeeler43 wants to merge 1 commit into
base: main
Choose a base branch
from
+8 −8
Open

fix(thermoelastic2d): align column names with v1 dataset #247

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions engibench/problems/thermoelastic2d/model/fea_model.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ def run(self, bcs: dict[str, Any], x_init: np.ndarray | None = None) -> dict[str
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.
Comment on lines 119 to 123
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copilot uses AI. Check for mistakes.
- 'force_elements_x' (np.ndarray): NxN binary array encoding the location of loaded elements in the x direction.
- 'force_elements_y' (np.ndarray): NxN binary array encoding the location of loaded elements in the y direction.
Expand All @@ -146,7 +146,7 @@ def run(self, bcs: dict[str, Any], x_init: np.ndarray | None = None) -> dict[str
nelx = fe_h - 1
nely = fe_w - 1

volfrac = bcs["volfrac"]
volfrac = bcs["volume_fraction_target"]
n = nely * nelx # Total number of elements
Comment on lines 147 to 150
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.

# OptiSteps records
Expand Down Expand Up @@ -285,7 +285,7 @@ def run(self, bcs: dict[str, Any], x_init: np.ndarray | None = None) -> dict[str
return {
"structural_compliance": f0valm,
"thermal_compliance": f0valt,
"volume_fraction": vf_error,
"volume_fraction_error": vf_error,
}
Comment on lines 285 to 289
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copilot uses AI. Check for mistakes.
vf_error = np.abs(np.mean(x) - volfrac)
obj_values = np.array([f0valm, f0valt, vf_error])
Expand Down Expand Up @@ -353,7 +353,7 @@ def run(self, bcs: dict[str, Any], x_init: np.ndarray | None = None) -> dict[str
"bcs": bcs,
"structural_compliance": f0valm,
"thermal_compliance": f0valt,
"volume_fraction": vf_error,
"volume_fraction_error": vf_error,
"opti_steps": opti_steps,
}

Expand All @@ -372,7 +372,7 @@ def run(self, bcs: dict[str, Any], x_init: np.ndarray | None = None) -> dict[str
"fixed_elements": [lci[21], lci[32], lci[43]],
"force_elements_y": [bri[31]],
"heatsink_elements": [lci[31], lci[32], lci[33]],
"volfrac": 0.2,
"volume_fraction_target": 0.2,
"rmin": 1.1,
"weight": 1.0, # 1.0 for pure structural, 0.0 for pure thermal
}
Expand Down
6 changes: 3 additions & 3 deletions engibench/problems/thermoelastic2d/v0.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class ThermoElastic2D(Problem[npt.NDArray]):
objectives: tuple[tuple[str, ObjectiveDirection], ...] = (
("structural_compliance", ObjectiveDirection.MINIMIZE),
("thermal_compliance", ObjectiveDirection.MINIMIZE),
("volume_fraction", ObjectiveDirection.MINIMIZE),
("volume_fraction_error", ObjectiveDirection.MINIMIZE),
)

@dataclass
Expand All @@ -65,7 +65,7 @@ class Conditions:
default_factory=lambda: HEATSINK_ELEMENTS
)
"""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
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
"""Target volume fraction for the volume fraction constraint"""
rmin: Annotated[
float, bounded(lower=1.0).category(THEORY), bounded(lower=0.0, upper=3.0).warning().category(IMPL)
Expand Down Expand Up @@ -129,7 +129,7 @@ def simulate(self, design: npt.NDArray, config: dict[str, Any] | None = None) ->
boundary_dict[key] = value

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"]])
Comment on lines 131 to +132
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.

def optimize(
self, starting_point: npt.NDArray, config: dict[str, Any] | None = None
Expand Down
Loading