ASK/TELL DEVELOP

[jlnav]

See checklist in #1307 (review)

[codecov]

Codecov Report

Attention: Patch coverage is 86.81055% with 55 lines in your changes missing coverage. Please review.

Project coverage is 77.46%. Comparing base (9f3f192) to head (aca1d50).

Files with missing lines	Patch %	Lines
libensemble/utils/runners.py	83.90%	10 Missing and 4 partials ⚠️
libensemble/gen_classes/aposmm.py	80.88%	4 Missing and 9 partials ⚠️
libensemble/generators.py	87.75%	6 Missing and 6 partials ⚠️
libensemble/utils/misc.py	91.48%	5 Missing and 3 partials ⚠️
libensemble/sim_funcs/borehole_kills.py	0.00%	5 Missing ⚠️
libensemble/gen_classes/sampling.py	97.14%	1 Missing ⚠️
libensemble/libE.py	0.00%	0 Missing and 1 partial ⚠️
libensemble/utils/validators.py	88.88%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #1307      +/-   ##
===========================================
+ Coverage    76.97%   77.46%   +0.49%     
===========================================
  Files           77       80       +3     
  Lines         7812     8184     +372     
  Branches      1160     1223      +63     
===========================================
+ Hits          6013     6340     +327     
- Misses        1598     1623      +25     
- Partials       201      221      +20     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[shuds13]

Does this need to be separate to constructor?

[jlnav]

Perhaps you've seen already, but setup() sets attributes that can't be pickled, but need to be done anyway. So I believe a separate generator.setup() needs to exist.

But perhaps for outside-of-libE purposes, to save a line, this could resemble: my_APOSMM = APOSMM(gen_specs, setup=True).

[shuds13]

@jlnav

Currently this has two existing user functions refactored to ask/tell, and it is a breaking change for codes using those gens. I don't think this is what we want. So, for now, we could supply these as duplicates (keeping the original), or remove them. Alternative would be to refactor all appropriate gens to ask/tell, but that would hold this up, so I think it may be better to supply these two as a duplicate for now.

Also, would it be better for these ask/tell gens (rand sample and gpCAM) to use AskTellGenRunner and not use the wrapper gen_f (given a breaking change anyway).

Issues we need to address:

• cleanup/resolve ask/tell refactoring - clear separation of gens - see branch Making new gpCAM gen class #1316
• Alt. could be to make old imports use the ask/tell gens?
• re-test runs with libE and Optimas that previously failed with threads
• compare running with wrapper and ask/tell runner - compare results
  - results match. ask/tell runner about 10% slower than wrapper for randsample test.
• CI passing
• Parameters in wrapper were rearranged (???) - requires discussion (for now changing on Making new gpCAM gen class #1316) - deciding this is another reason why we may not want to replace the old gen_f's yet.
• test_1d_asktell_gen.py is not 1d, its 2d - changing on Making new gpCAM gen class #1316
• are we now getting reproducible/matching random numbers in APOSMM ask/tell test - check seeding (inc. when running via Optimas).
• Need assert on minima found in aposmm test - if losing precision - why.
• check comparison using diff alloc for APOSMM - including initial sample processing.

[jlnav]

@jlnav

Currently this has two existing user functions refactored to ask/tell, and it is a breaking change for codes using those gens. I don't think this is what we want. So, for now, we could supply these as duplicates (keeping the original), or remove them. Alternative would be to refactor all appropriate gens to ask/tell, but that would hold this up, so I think it may be better to supply these two as a duplicate for now.

Sounds good. Maybe we could raise DeprecationWarnings?

Also, would it be better for these ask/tell gens (rand sample and gpCAM) to use AskTellGenRunner and not use the wrapper gen_f?

I prefer the AskTellGenRunner myself (I did develop it), but if all the gens move to my runner, then what would you want to do with your wrapper?

[shuds13]

I think there is also some opportunity for inheritence with the gpCAM class. And gp_cam_simple is more complicated than gp_cam_asktell. So, if anything the latter would be the base class, and renamed to avoid confusion. See #1316

[shuds13]

Consequences of removing the gen_f wrapper.
Orchestration, in terms of how you interface with a model, happens inside a gen_f.
This includes things like where we decide resources for each simulation.
For example, in the Optimus gen_f, it has an option to set num_procs or num_gpus for each simulation. This could be used with any generator . If we get rid of the gen_f wrapper where does the user specify this?

When we bring up things like wanting to examine our model at the end, the answer is that it's orchestration and so should not be in a generator. The only place we have to do these things is inside a gen_f.

[shuds13]

For libEnsemble arguments this is opaque when looking at this routine. Perhaps they should be written out as additional arguments. Then there's the question of whether you keep gen_specs as well.