Added DTLZ multiobjective test suite, based on modification of ParMOO test suite #8
Conversation
…rface, modified hpo file to call dtlz suite
| @@ -0,0 +1 @@ | |||
| __version__ = "0.0.1" | |||
There was a problem hiding this comment.
I suggest we stick to the version attribute of the Benchmark class defined in benchmark.py.
There was a problem hiding this comment.
It is also there, (name in the class is version, not __version__. Should I just delete this occurrence?
There was a problem hiding this comment.
I think a link is missing in the "there". for benchmarks let's keep it as a class attribute version. For Python packages (e.g., deephyper, deephyper_benchmarks) it should be __version__ in the root __init__.py of the package.
lib/DTLZ/benchmark.py
Outdated
| DIR = os.path.dirname(os.path.abspath(__file__)) | ||
|
|
||
|
|
||
| class DTLZ_lib(Benchmark): |
There was a problem hiding this comment.
For class's names follow camel cases: "Start each word with a capital letter. Do not separate words with underscores. This style is called camel case or pascal case." No underscore, DTLZBenchmark maybe.
There was a problem hiding this comment.
OK DTLZBenchmark is fine -- IMHO, camel case doesn't work well when using acronyms like DTLZ, but perhaps it is better to maintain consistency
There was a problem hiding this comment.
yeah, mainly for consistency see MPIDistributedBO, LCModelStopper.
lib/DTLZ/model.py
Outdated
There was a problem hiding this comment.
Nothing to say about the content of the file! I also created some model.py files in neural network benchmarks where I define the model. If we reuse this model name of "utility modules" maybe we should document it in the main README.
|
|
||
| # Read problem dims and definition (or read from ENV) | ||
| nb_dim = int(os.environ.get("DEEPHYPER_BENCHMARK_NDIMS", 5)) | ||
| nb_obj = int(os.environ.get("DEEPHYPER_BENCHMARK_NOBJS", 2)) |
There was a problem hiding this comment.
I am wondering how we could be consistent between what should be "Environment variables" (e.g., DEEPHYPER_BENCHMARK_NDIMS) and "parameters of the run-function (e.g., sleep=False). Maybe the sleep=False, sleep_mean=..., sleep_noise=... should be implemented through a "decorator so that any function can be extended and we code it only once.
There was a problem hiding this comment.
OK, if you have a decorator I'm happy to use it. For now, I had just copy-pasted the hpo definition from C-BBO.ackley and changed the function import/call to match this directory.
For the environment variables, I tried to copy the style that you used in Ackley. Open to changes if you have suggestions. For hyperparams that are specific to DTLZ, I added a _DTLZ_ in the middle of the variable name
| @@ -0,0 +1,50 @@ | |||
| import os | |||
There was a problem hiding this comment.
performing sorting of imports would be nice (proposed in visual studio code if you use this editor).
There was a problem hiding this comment.
OK, I don't like fancy editors -- it's all vim and CL for me, but I think that's reasonable.
My only suggestion would be that I think it's nice to keep intrinsics (such as os and time) separate from external libraries (like numpy) separate from properties of this package (e.g., deephyper imports)
I'm fine with doing everything alphabetically to keep the style of deephyper consistent, but for now I've just sorted within those three classes. Let me know if you'd like it sorted universally
There was a problem hiding this comment.
Yes, it is not 100% alphabetical sorting, see here for example
- alphabetical ordering within each sub-group
importbeforefrom ... import ...- standard libraries before external dependencies
- deephyper imports last
|
Ooo, @Deathn0t , one additional question about this that I forgot to ask in the PR message: When running Why is this? I'm not using tensorflow for anything and my understanding of deephyper is that it shouldn't need tensorflow unless it is specifically doing a tensorflow model's hyperparameter optimization, but I could be mistaken |
|
Hey @thchang , yeah good point and good question. So, when running It would be better to have a lazy loader, trying to import |
Modified the test problems in ParMOO's sample simulation library, to fit with DeepHyper and provided an example script importing and solving DTLZ2.
Addresses Issue #5