-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
1810 fix #1825
1810 fix #1825
Conversation
My test take forever with script below and eventual throw error:
idmtools_slurm.ini:
|
test_id.py.txt
I kind of know where 26 come from: |
…d on Sharon's feedback
* Ensure slurm uses uuid
* add configuration caching for id generation * add performance test scaler
|
idmtools_1.ini
Run following code by delete DEST folder first, always get this error:
|
…irectory_map.py to avoid confusion
…on metadata in file. & linting fixes
@@ -231,3 +234,9 @@ def post_run_item(self, experiment: Experiment, **kwargs): | |||
sim_path = Path(exp_path, "simulation_index.json") | |||
with open(sim_path, "w") as f: | |||
json.dump([s.id for s in experiment.simulations], f) | |||
|
|||
if IdmConfigParser.get_option(None, "id_generator", "uuid").lower() == "item_sequence": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the almost exactly what super class function of post_run_item does (still missing experiment.post_run(self.platform). so no need to copy code. just need call super function.
idmtools/idmtools_platform_comps/idmtools_platform_comps/comps_operations/experiment_operations.py Line 44 in e3ec545
idmtools/idmtools_platform_comps/idmtools_platform_comps/comps_operations/simulation_operations.py Line 114 in e3ec545
idmtools/idmtools_platform_comps/idmtools_platform_comps/comps_operations/suite_operations.py Line 29 in e3ec545
Line 35 in e3ec545
Line 35 in e3ec545
You still have lot of places using UUID for id as above lines. Do you intend to do that? |
idmtools_test/idmtools_test/utils/execute_operations/experiment_operations.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested the latest fixes. Even there are still few issues(like can not random print id, too fridge if let user define sequence file), but overall feature is good.
Maybe we should consider to release to a separate branch instead of dev branch until we are sure to release this feature. Meanwhile we can keep develop on dev branch and release new stuff
Can we fix conflicts? @emilykclaps |
@shchen-idmod just resolved conflicts. Where are we with this PR? Can we merge to dev, or did you want to release to separate branch until ready to merge? Thx! |
Add beta feature warning
FAILED test_slurm_cancel1.py::TestSlurmCanceling::test_canceling_a_full_experiment - NameError: name 'meta' is not defined |
# Conflicts: # idmtools_platform_slurm/idmtools_platform_slurm/platform_operations/experiment_operations.py
There is known issue about build-docs. Other than that everything looks good |
Addresses core issues of #1810
This PR incorporates id-generation plugins for item ID's. The name of the plugin the user desires to use can be specified in the .ini file under [COMMON] > 'id_generator'. The 2 plugins we defined are:
Documentation and tests were also added for this feature.
*Note - the id sequential generator plugin is intended to be used with Slurm, not with COMPS.
To support this, we also added tickets related
#1930
#1936
#1937
#1938
#1939
#1954