Skip to content

fix: robustify MOSEGO constraint relaxation and expand convergence tests - #24

Merged
oihanc merged 6 commits into
SMTorg:mainfrom
Paul-Saves:feat/improving-mosego
Jul 21, 2026
Merged

oihanc merged 6 commits into
SMTorg:mainfrom
Paul-Saves:feat/improving-mosego

Conversation

@Paul-Saves

@Paul-Saves Paul-Saves commented Jul 12, 2026

Copy link
Copy Markdown
Member

This Pull Request finalizes and robustifies the MOSEGO acquisition strategy for multi-objective constrained optimization. It focuses on mathematically correcting the constraint relaxation, enforcing strict seeding compliance, expanding the test suite, and enhancing the notebooks.

• Constraint Relaxation Refactoring
• The relax_constraints parameter has been upgraded from a rigid boolean to a continuous float (default 0.0 ). It now algebraically controls the variance margins applied to constraints (μ ± r × σ).
• Streamlined build_scipy_constraints logic by removing obsolete conditionals for inequality bounds, making the code mathematically precise.
• Extensive Convergence & Unit Testing
• Added hypervolume growth convergence tests for ZDT1 using both EHVI and MPI .
• Added robust constrained convergence tests for BNH ensuring MOSEGO performs flawlessly with both dynamic relaxation ( relax_constraints=2.0 ) and hard constraints ( 0.0 ).
• Substantial unit test coverage added for all MO benchmarks and acquisition functionalities.
• Notebook Enhancements ( multi_obj.ipynb )
• Added visual scatter plots comparing the effects of relax_constraints=2.0 vs 0.0 on the BNH problem (highlighting feasible/infeasible regions).
• Added Pareto front comparisons between EHVI and MPI .

[✓] Code successfully passes ruff check on both src/ and src/tests/
[✓] All Pytest convergence and unit tests pass
[✓] Example notebooks are fully functional and clean

@Paul-Saves Paul-Saves changed the title improve_mosego feat: robustify MOSEGO constraint relaxation and expand convergence tests Jul 12, 2026
@coveralls

coveralls commented Jul 12, 2026

Copy link
Copy Markdown

Coverage Report for CI Build 29764570155

Coverage increased (+0.3%) to 87.913%

Details

  • Coverage increased (+0.3%) from the base build.
  • Patch coverage: 4 uncovered changes across 4 files (193 of 197 lines covered, 97.97%).
  • 3 coverage regressions across 1 file.

Uncovered Changes

File Changed Covered %
src/tests/test_unit/test_ehvi.py 26 25 96.15%
src/tests/test_unit/test_mo_benchmarks.py 43 42 97.67%
src/tests/test_unit/test_mpi.py 33 32 96.97%
src/tests/test_unit/test_zdt.py 38 37 97.37%
Total (12 files) 197 193 97.97%

Coverage Regressions

3 previously-covered lines in 1 file lost coverage.

File Lines Losing Coverage Coverage
src/smt_optim/subsolvers/multistart.py 3 47.58%

Coverage Stats

Coverage Status
Relevant Lines: 5204
Covered Lines: 4575
Line Coverage: 87.91%
Coverage Strength: 1.76 hits per line

💛 - Coveralls

@Paul-Saves
Paul-Saves force-pushed the feat/improving-mosego branch from 93c1bd7 to b873a4b Compare July 12, 2026 03:57
@Paul-Saves
Paul-Saves requested a review from oihanc July 12, 2026 03:57
@Paul-Saves
Paul-Saves force-pushed the feat/improving-mosego branch from 7408ac4 to 7a13d4b Compare July 12, 2026 04:09
@Paul-Saves
Paul-Saves force-pushed the feat/improving-mosego branch from 131abfc to 0f83ef7 Compare July 12, 2026 04:39

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Although I believe checking for negative variance before computing the square root would be more numerically stable, theoretically, one objective variance could be less than 1e-12 while the other is not. In that case, the EHVI value would not necessarily be equal to 0.
For example, considering the following values:

Y = np.array([
    [3, 1],
    [2, 1.5].
    [1, 2.5],
])

mu = np.array([2, 1.5])
s = np.array([0.7, 1e-12])

The EHVI value is 0.315, even though s is below 1e-6 (sqrt(1e-12)). Furthermore, variance values below 1e-12 are common in the MF context. I do find the limit quite high.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Since the CDF is multiplied here, it is indeed possible to skip the MPI calculation, when the variance is too low. On that note, I just realized that the return condition when the variance is null should be 0, not 1 (error was already present prior to this PR). However, I still believe that the tolerance of 1e-12 is too high, especially in the MF context.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

There are already very similar tests in tests/test_integration/test_mosego for:

  • test_zdt1_mosego_ehvi_hypervolume_growth

  • test_dtlz5_mosego_multifidelity_constrained

For tests not requiring multi-fidelity, I would recommend using SmtGPX, since it is much faster (except if the purpose was to test MO-SEGO in the mono-fidelity context with the KRG model).

For test_dtlz5_mosego_multifidelity_constrained, the assert condition does not verify the same condition as mentioned in the comment, as the export_as_dict class method returns sample data from every fidelity level.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

If the relax argument is set to 0, I think we should avoid computing the variance as to limit the computation cost. (when dealing with lower or upper value constraints).

@Paul-Saves
Paul-Saves requested a review from oihanc July 13, 2026 14:47
@Paul-Saves

Copy link
Copy Markdown
Member Author

Thank you so much @oihanc for the work! I've updated the code!

@Paul-Saves Paul-Saves changed the title feat: robustify MOSEGO constraint relaxation and expand convergence tests fix: robustify MOSEGO constraint relaxation and expand convergence tests Jul 14, 2026
@oihanc

oihanc commented Jul 15, 2026

Copy link
Copy Markdown
Collaborator

Hi Paul,

Thanks for the fixes and for continuing to improve MO-SEGO! I had a couple of questions regarding the unit tests.

I'm not sure I fully understand the distinction between test_zdt1_hv.py::test_zdt1_mosego_hv and test_mosego.py::test_no_constraint. As far as I can tell, both tests verify MOSEGO on the ZDT1 problem. I noticed that test_zdt1_mosego_hv also asserts that the dataset is populated and that the hypervolume is greater than 0. Would it make sense to add those assertions to the existing test instead of having a separate one? Or is there a specific behavior that test_zdt1_mosego_hv is intended to cover?

Also, would it make sense to move test_bnh_mosego_constrained_convergence and test_bnh_mosego_constrained_convergence_no_relax into test_integration/test_mosego.py, since they all cover MOSEGO? It seems like that might make the test organization a bit more consistent, but I'm curious to hear your thoughts.

I had originally envisioned the test_convergence directory as a place for benchmarking the convergence properties of the optimization algorithms implemented in SMT-optim. However, I ended up not pursuing the idea because those tests are relatively expensive to run. That's why I was wondering whether grouping the MOSEGO tests under test_integration/test_mosego.py might be a more consistent organization.

Let me know what you think!

@Paul-Saves
Paul-Saves force-pushed the feat/improving-mosego branch from 003e8d5 to d8776b8 Compare July 20, 2026 17:40
@Paul-Saves

Copy link
Copy Markdown
Member Author

@oihanc I've reduced the number of tests and moved them to reduce the computational time

@oihanc oihanc left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks, @paul, for your contribution and for helping make MO-SEGO more robust!

@oihanc
oihanc merged commit a302470 into SMTorg:main Jul 21, 2026
5 checks passed
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.

3 participants