Consolidate WLM functions #199
Conversation
Merge branch 'develop' of https://github.com/MattToast/SmartSim into cons-wlm-funcs
Merge branch 'develop' into cons-wlm-funcs
conftest.py
Outdated
| @pytest.fixture(scope="session") | ||
| def alloc_specs(): | ||
| specs = {} | ||
| spec_sheet_path = os.getenv("SMARTSIM_TEST_ALLOC_SPEC_SHEET_PATH", None) |
There was a problem hiding this comment.
There is a function in this file which prints the value of all SMARTSIM_TEST_... variables. Can you add this variable to it? It will be useful for logging purposes.
There was a problem hiding this comment.
Good call! I remember seeing that and then totally forgot to add the new envvar there.
Added SMARTSIM_TEST_ALLOC_SPEC_SHEET_PATH to print_test_configuration at 32e2378
| "`smartsim.slurm` has been deprecated and will be removed in a future release.\n" | ||
| "Please update your code to use `smartsim.wlm.slurm`" | ||
| ) | ||
| warn(msg, category=DeprecationWarning, stacklevel=2) |
There was a problem hiding this comment.
As stacklevel=2 is correct, I vaguely remember you pointing out in some other portions of the code, this was set incorrectly. Would it be possible for you to go and update those occurrences? Thanks!
There was a problem hiding this comment.
No problem! Added at 32e2378
| @@ -0,0 +1,118 @@ | |||
| import os | |||
There was a problem hiding this comment.
Not to be overly nitpicking, but can we use a more descriptive name for this file? Something more along the lines of test_wlm_helper_functions.py? I think consolidation_functions is defined within the scope of this PR, but not too much elsewhere.
There was a problem hiding this comment.
Not overly nitpicking at all! I was a little weary on the name myself. I like the name test_wlm_helper_functions.py as is so I changed it to that!!
Rename test_consolidation_functions.py -> test_wlm_helper_functions.py at 32e2378
- Update additional calls to warn w/ stacklevel=2 - Print the used alloc spec sheet path - Give a better name for the new test file
Codecov Report
@@ Coverage Diff @@
## develop #199 +/- ##
===========================================
+ Coverage 83.02% 84.17% +1.15%
===========================================
Files 58 58
Lines 3263 3242 -21
===========================================
+ Hits 2709 2729 +20
+ Misses 554 513 -41
|
This PR consolidates user facing functions specific to a workload manager in a new
smartsim.wlmmodule. This is done by:smartsim.slurm. Now when this module is imported a depreciation warning is displayed. All functionality previously there has been moved tosmartsim.wlm.slurmget_hosts,get_queue,get_tasks, andget_tasks_per_node. These can be accessed insmartsim.wlm.slurmandsmartsim.wlm.pbsrespectively.smartsim.wlm:get_hosts,get_queue,get_tasks, andget_tasks_per_nodemethods have also been created to which will detect and then use the appropriate implementation (slurm/PBS) to allow for portable SmartSim scripts.SMARTSIM_TEST_ALLOC_SPEC_SHEET_PATHenvironment variable to a path to a json file containing information about the current allocation. Example spec sheets are found intests/test_configs/alloc_spec_sheet_examples/. If this variable is not set or a key is missing from the allocation spec sheet, the test suite will simply call the methods and ensure that no exceptions arise.