Implementation of ZOMBI.activate

[knstntn-ts]

I noticed something that could be a typo in the implementation of the ZOMBI.activate function:

ZoMBI/zombi.py

Line 109 in 7f46791

fX_bounded = np.append(fX_new, self.dataset_fX[idx])

I believe the purpose is to get the fX value from dataset_fX at X after optimizing the acquisition function. The way the code is written now does so by looking up a value in dataset_fX at the index idx. However, since this idx is found from ac_value, which itself is optimized within the bounded_norm, bounded_norm[idx] may not correspond to the same X in the dataset_X, i.e., it may be the case that bounded_norm[idx] != dataset_X[idx]. This may result in the wrong fX value that is used in the optimization.

Alternative implementation

I replaced the original version with fX_bounded = np.append(fX_new, -1*self.fX_model(bounded_norm[idx])) (direct evaluation of the target function at optimum X) and compared the results.
Here are the results (mean of 10 runs) of optimizing the Negative Poisson ratio:

It seems that the alternative implementation has better performance.

Reproducing the results

Attached is the notebook that I used in generating the aforementioned results. I moved ZOMBI class outside of the zombi.py file into the Jupyter notebook. The code in utils.py and acquisitions.py is the same with the original version.
ZOMBI_test.zip

Discussion

Did I read misunderstand the code? If so, then I am curious about your thoughts on why the alternative implementation yielded better performance. Let me know what you think.

[aleks-py]

Thank you for your thorough review of my code! You are in fact correct that in its current implementation, idx may produce inconsistencies. I have reviewed your .ipynb file as well as attempted to implement your suggestion of fX_bounded = np.append(fX_new, -1*self.fX_model(bounded_norm[idx])).

I confirm that I get the same results as you on my machine. I have made the appropriate fixes to the code and updated the repo. Thanks again, and let me know if you spot any other inconsistencies 😄

[buonassisi]

Thank you, all!--Tonio BuonassisiProf. Mech. Eng., MIT✉️ ***@***.***💻 http://www.buonassisigroup.com🤝 https://www.LinkedIn.com/today/author/tonio-buonassisi-7854543On Feb 14, 2024, at 14:06, Aleks ***@***.***> wrote: Thank you for your thorough review of my code! You are in fact correct that in its current implementation, idx may produce inconsistencies. I have reviewed your .ipynb file as well as attempted to implement your suggestion of fX_bounded = np.append(fX_new, -1*self.fX_model(bounded_norm[idx])). I confirm that I get the same results as you on my machine. I have made the appropriate fixes to the code and updated the repo. Thanks again, and let me know if you spot any other inconsistencies 😄 —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>