Skip to content

fix v0.py warm-start and remove redundant calcs in optimize#210

Merged
arthurdrake1 merged 8 commits into
mainfrom
fix/beams2d-warmstart
Dec 19, 2025
Merged

fix v0.py warm-start and remove redundant calcs in optimize#210
arthurdrake1 merged 8 commits into
mainfrom
fix/beams2d-warmstart

Conversation

@arthurdrake1
Copy link
Copy Markdown
Contributor

@arthurdrake1 arthurdrake1 commented Dec 11, 2025

Description

Fixes a warm-start issue where disconnected beam members in generated samples failed to be connected by the optimizer. This requires a shift to beams2d v1 as final optimization results are directly impacted. However, the v0 dataset remains unchanged and is thus manually referenced by v1.

Type of change

  • New feature (non-breaking change which adds functionality)

Checklist:

  • I have run the pre-commit checks with pre-commit run --all-files
  • I have run ruff check . and ruff format
  • I have run mypy .
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@arthurdrake1
Copy link
Copy Markdown
Contributor Author

arthurdrake1 commented Dec 12, 2025

@g-braeunlich We are attempting a move to beams2d v1 with a minor warm-start fix. Please see the exact changes via git diff --no-index ./engibench/problems/beams2d/v0.py ./engibench/problems/beams2d/v1.py.

The main change: I replaced x = deepcopy(starting_point) with

eps = 1e-4
x = (
    (1 - eps) * starting_point + eps * base_config.volfrac
)  # add tiny non-zero values to avoid warm-start gradient issues for zero values

and also removed a few redundant calculations of dc, dv, and ce at the start of the optimization in that same part of the code. Everything else is for compatibility/documentation purposes.

Copy link
Copy Markdown
Collaborator

@g-braeunlich g-braeunlich left a comment

Choose a reason for hiding this comment

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

How about inheriting from v0 and only change differing attributes / methods (simulate)?

@markfuge
Copy link
Copy Markdown
Member

@g-braeunlich I agree with this in principle. I think @ffelten wanted to keep versions of problems self contained in each file for readability and pedagogical purposes, but I don't know if that was for EngiBench or just for EngiOpt, since the latter was more for instructional/learning purposes. Perhaps @ffelten can weigh in

@ffelten
Copy link
Copy Markdown
Collaborator

ffelten commented Dec 15, 2025

How about inheriting from v0 and only change differing attributes / methods (simulate)?

I believe this makes sense.

EngiOpt is a different beast that has some pedagogical goals, utilizing single-file implementations when possible. In EngiBench, I believe we can rely on inheritance or static functions to avoid code duplication, as we are not really aiming to be a simulation lab.

@markfuge
Copy link
Copy Markdown
Member

Great then I agree with @g-braeunlich on the suggestion

@arthurdrake1
Copy link
Copy Markdown
Contributor Author

arthurdrake1 commented Dec 15, 2025

@g-braeunlich What should we do about version since v1.py should still use the original dataset? Do we need to define a new dataset_version attribute in core.py? Right now I notice that across all problems, the only practical use of the version variable is for data loading which could become problematic later

@g-braeunlich
Copy link
Copy Markdown
Collaborator

@ffelten what do you think?

@ffelten
Copy link
Copy Markdown
Collaborator

ffelten commented Dec 16, 2025

@g-braeunlich What should we do about version since v1.py should still use the original dataset? Do we need to define a new dataset_version attribute in core.py? Right now I notice that across all problems, the only practical use of the version variable is for data loading which could become problematic later

I believe dataset_id in v1.py can point to the dataset v0, i.e. no change from v0.py? This is the string that is used to pull the dataset from HF.

@arthurdrake1
Copy link
Copy Markdown
Contributor Author

@g-braeunlich What should we do about version since v1.py should still use the original dataset? Do we need to define a new dataset_version attribute in core.py? Right now I notice that across all problems, the only practical use of the version variable is for data loading which could become problematic later

I believe dataset_id in v1.py can point to the dataset v0, i.e. no change from v0.py? This is the string that is used to pull the dataset from HF.

I guess this is the other option, that we just hardcode the dataset version as in dataset_id = f"IDEALLab/beams_2d_{Config.nely}_{Config.nelx}_v0". But then version becomes entirely unused. Should I then remove the version definition or keep it in case we need it for a future change?

@arthurdrake1
Copy link
Copy Markdown
Contributor Author

@g-braeunlich After further conversation we decided to still define version for future reference and hard-code the v0 reference in dataset_id. Please check if I have inherited v1 properly as well. Thanks!

@g-braeunlich
Copy link
Copy Markdown
Collaborator

OK. That looks good to me. On also could think of sharing the __main__ section by defining a main function (which then would take a problem as an argument to not accidentally loading v0 instead of v1).
It then would be

if __name__ == "__main__":
    main()

@arthurdrake1
Copy link
Copy Markdown
Contributor Author

OK. That looks good to me. On also could think of sharing the __main__ section by defining a main function (which then would take a problem as an argument to not accidentally loading v0 instead of v1). It then would be

if __name__ == "__main__":
    main()

Thanks for the suggestion – this is done.

@g-braeunlich
Copy link
Copy Markdown
Collaborator

Are you sure, that the main() in v1 also loads the problem defined in v1 and not that one defined in v0?
I would guess that it loads v0

@g-braeunlich
Copy link
Copy Markdown
Collaborator

Just tested it. And for me, main() loads v0 even if it is used in v1.
I guess my suggestion was incomplete. I should have added the argument to main:

if __name__ == "__main__":
    main(Beams2D)

And it also should be:

def main(problem_type: type[Problem]):
    """Provides a way to instantiate the problem without having to pass configs to optimize or simulate later.

    Possible sets of nely and nelx: (25, 50), (50, 100), and (100, 200)
    If a new nely and nelx are not passed in, uses the default conditions.
    """
    problem = problem_type(seed=0)

@arthurdrake1
Copy link
Copy Markdown
Contributor Author

@g-braeunlich Yes, seems like I missed this minor detail. I tested it and the v0, v1 main() calls are distinct now.

@arthurdrake1 arthurdrake1 merged commit 4cb9d57 into main Dec 19, 2025
14 checks passed
@arthurdrake1 arthurdrake1 deleted the fix/beams2d-warmstart branch December 19, 2025 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants