Skip to content

Conversation

@RemiLehe
Copy link
Collaborator

@RemiLehe RemiLehe commented Oct 31, 2024

Overall approach:

optimas generators currently call the ask and tell methods, which, under the hood, can the _ask and _tell methods.

In this PR:

  • ask/tell are renamed to ask_trials/tell_trials and keep the same interface (for compatibility with the rest of the framework)
  • _ask/_tell are renamed to ask/tell and adopt the new standardized interface.

This allows other packages to use the optimas generators. (In this case, these other packages will call the new, standardized ask/tell directly, without going through ask_trials/tell_trials.)

In order to wrap a standardized generator from another package, one would only need to make a thin wrapper around it that defines ask_trials and tell_trials.

Checks (SH):

  • Check output of multitask trials
  • Changed TrialParameter from trial_index to ax_trial_id
  • Before _, trial_id = self._ax_client.attach_trial(params) could get called on an ignored trial, is this needed?
  • Can we "tell" all ready points or must we keep with telling one at a time (as currently does)?
  • tell_trials function can work on a set of points, but if do more than one at a time...
  • Saving points to file happens after the batch so does not exactly match requested frequency
  • If more than max_evals done, rest of batch gets given (this could be truncated with an internal check if they are not wanted).
  • More efficient to tell all ready points (small diff but consider when using a remote gen - like in APOSMM case).

RemiLehe and others added 30 commits July 10, 2024 07:58
@shuds13
Copy link
Member

shuds13 commented May 2, 2025

Check output of multitask trials

./print_fields.py tests_output/ax_mt_history.npy -f sim_id trial_index ax_trial_index ax_arm_name

Original (from main)

2 runs:

[( 0,  0, 0, '0_0')
 ( 1,  1, 0, '0_1')
 ( 2,  2, 0, '0_2')
 ( 3,  3, 0, '0_3')
 ( 4,  4, 0, '0_4')
 ( 5,  5, 1, '1_0')
 ( 6,  6, 1, '1_1')
 ( 7,  7, 2, '2_0')
 ( 8,  8, 2, '2_1')
 ( 9,  9, 2, '2_2')
 (10, 10, 3, '2_2')
 (11, 11, 4, '4_0')
 (12, 12, 4, '4_1')
 (13, 13, 4, '4_2')
 (14, 14, 5, '4_0')]

[( 0,  0, 0, '0_0')
 ( 1,  1, 0, '0_1')
 ( 2,  2, 0, '0_2')
 ( 3,  3, 0, '0_3')
 ( 4,  4, 0, '0_4')
 ( 5,  5, 1, '1_0')
 ( 6,  6, 1, '1_1')
 ( 7,  7, 2, '2_0')
 ( 8,  8, 2, '2_1')
 ( 9,  9, 2, '2_2')
 (10, 10, 3, '2_0')
 (11, 11, 4, '4_0')
 (12, 12, 4, '4_1')
 (13, 13, 4, '4_2')
 (14, 14, 5, '4_2')]

New

[( 0,  0, 0, '0_0')
 ( 1,  1, 0, '0_1')
 ( 2,  2, 0, '0_2')
 ( 3,  3, 0, '0_3')
 ( 4,  4, 0, '0_4')
 ( 5,  5, 1, '1_0')
 ( 6,  6, 1, '1_1')
 ( 7,  7, 2, '2_0')
 ( 8,  8, 2, '2_1')
 ( 9,  9, 2, '2_2')
 (10, 10, 3, '2_2')
 (11, 11, 4, '4_0')
 (12, 12, 4, '4_1')
 (13, 13, 4, '4_2')
 (14, 14, 5, '4_2')]

Also note that the multi-task generator - suggest often returns fewer than the points requested, which by the standard would result in raising an error. This is because points are generated in groups (a given ax trial id) which are buffered internally - if the number asked for does not align, you get short batches at the end of each group (the new coding was initially also done in a way that resulted in multiple of the same point being returned in this case, but I assume it was not by design but wrong indentation?)

This also brings up the question why Optimas limits generation to the number of workers, rather than just giving libEnsemble all points produced, and letting it manage the queue. This would also prevent the need to have failed suggests.

The multitask test in test_ax_generators.py generation resembles:

  • generators lofi batch (5 points) ax_trial_id 0: arm id 0_*.
  • hifi batch (2 points) ax_trial_id 1: arm id 1_*.
  • lofi batch (3 points) ax_trial_id 2: arm id 2_*.
  • hifi batch (1 point) ax_trial_id 3: arm id 2_.
    lofi batch : arm id 4_
    .

I think I follow the pattern here. The second hifi batch is processing a best point chosen from the previous lofi batch (hence a repeated arm), but uses a new ax trial id for the batch. The arm name really IDs the point. So arm could be mapped to an _id. I think any unknown point with such an identifier should be given new id (to prevent potential duplication of ids).

Comment on lines 114 to 115
# Register trials with unknown SEM
# generator.tell_trials(trials) # Give as batch
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Should we switch to this code?
What was the intention for this comment?

Copy link
Member

Choose a reason for hiding this comment

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

I updated to batch, but it throws out a few things - like checkpointing at an exact id.

"GridSamplingGenerator",
"LineSamplingGenerator",
"RandomSamplingGenerator",
"ExternalGenerator",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Would it make sense to add a test for this object in the CI?
(This could potentially be a follow-up PR.)

Copy link
Member

Choose a reason for hiding this comment

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

yes

config[var.name] for var in trial.varying_parameters
]
return trials
points.append(config)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Suggested change
points.append(config)
points.append(config)

make_plots(gen)


def test_ax_single_fidelity_resume():
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It seems that this function was simply moved from its previous location in this file to this new location.

If this is indeed the case, could you move it back to its previous location?

Copy link
Member

Choose a reason for hiding this comment

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

It was the one function that was out of order from the list of tests - why should it be out of order?

@RemiLehe RemiLehe changed the title [WIP] Attempt to make optimas generators compatible with generator standard Attempt to make optimas generators compatible with generator standard May 28, 2025
# If want to use _id can have a mapping inside the generator, but those
# points that are part of same ax trial_index will not be seen in history.
custom_trial_parameters = [
TrialParameter("arm_name", "ax_arm_name", dtype="U32"),
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As a follow-up PR, let us consider how to fit this in the VOCS format and/or whether to convert arm_id as a unique _id.

@RemiLehe RemiLehe enabled auto-merge May 28, 2025 20:29
@RemiLehe RemiLehe merged commit 1dd3e5d into main May 28, 2025
9 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