Skip to content
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

Run executable from task dir #124

Merged
merged 2 commits into from
Aug 7, 2023
Merged

Conversation

SeanBryan51
Copy link
Collaborator

@SeanBryan51 SeanBryan51 commented Aug 4, 2023

Currently, the CABLE executable is run from the current working directory instead of the task directory by using the absolute path to the executable. However, we should be running the executable from the task directory (otherwise CABLE will complain it cannot find the soil and PFT namelist files). This was a bug that was introduced in #99.

This change makes it so we change into the task directory before running the CABLE executable.

Fixes #123

@SeanBryan51 SeanBryan51 linked an issue Aug 4, 2023 that may be closed by this pull request
@codecov
Copy link

codecov bot commented Aug 4, 2023

Codecov Report

Merging #124 (b5c2169) into master (49da522) will decrease coverage by 0.02%.
Report is 1 commits behind head on master.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #124      +/-   ##
==========================================
- Coverage   88.46%   88.44%   -0.02%     
==========================================
  Files          26       27       +1     
  Lines        1387     1385       -2     
==========================================
- Hits         1227     1225       -2     
  Misses        160      160              
Files Changed Coverage Δ
benchcab/repository.py 98.59% <100.00%> (-0.15%) ⬇️
benchcab/task.py 83.00% <100.00%> (ø)
benchcab/utils/os.py 100.00% <100.00%> (ø)
tests/test_task.py 100.00% <100.00%> (ø)

Currently, the CABLE executable is run from the current working
directory instead of the task directory by using the absolute path to
the executable. However, we should be running the executable from the
task directory (otherwise CABLE will complain it cannot find the soil
and PFT namelist files). This was a bug that was introduced in the #99.

This change makes it so we change into the task directory before running
the CABLE executable.

Fixes #123
@SeanBryan51 SeanBryan51 force-pushed the 123-run-cable-exe-from-task-dir branch from a9e731b to 2d1de0c Compare August 4, 2023 01:14
@SeanBryan51
Copy link
Collaborator Author

Integration tests

Test five site experiment:

#!/bin/bash
bench_example_dir='bench_example_test_five_site_experiment'
rm -rf $bench_example_dir
git clone git@github.com:CABLE-LSM/bench_example.git $bench_example_dir
cd $bench_example_dir
git reset --hard 6287539e96fc8ef36dc578201fbf9847314147fb
sed -i 's/project:.*/project: tm70/g' config.yaml
sed -i 's/experiment:.*/experiment: five-site-test/g' config.yaml
sed -i 's/ccc561\/v3.0-YP-changes/sb8430\/test-branch/g' config.yaml
benchcab run -v

CABLE standard output:

 THE NAME LIST IS cable.nml
 Use spatially-specific soil properties;          360         150
           0
           0
 When choosing spatially-specific soil properties,
 snow-free albedo is also overwritten by this data set.
 Reading CABLE_PFTPARM namelist...
 Reading CABLE_SOILPARM namelist...
 Total number of patches (countPatch):            1
 iveg           1           2
 patchfrac           1   1.000000    
  Could neither find restart file ./
  nor ./
  Pre-loaded default initialisations are used.
 Reading CABLE_PFTPARM namelist...
 Reading CABLE_SOILPARM namelist...
 
 time-space-averaged energy & water balances
 Ebal_tot[Wm-2], Wbal_tot[mm per timestep]  9.5453368E-07 -7.0809359E-03
 time-space-averaged latent heat and net photosynthesis
 sum_fe[Wm-2], sum_fpn[umol/m2/s]   61.4036706086355     
 -8.074845742925115E-005
 
 NB. Offline-serial runs spinup cycles:           1
 Internal check shows in this version new_sumbal != trunk sumbal
 Writing new_sumbal to the file:new_sumbal
 setting SPINON -> FALSE           0           1
 Finished.    80.06340      seconds needed for year
 Finished.    80.07153      seconds needed for       140256  hours

Copy link
Collaborator

@ccarouge ccarouge left a comment

Choose a reason for hiding this comment

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

I have a small question but I'm happy for this to go in either way.

@@ -295,7 +295,7 @@ def test_run_cable():

# Success case: run CABLE executable in subprocess
task.run_cable()
assert f"{exe_path} {nml_path}" in mock_subprocess.commands
assert f"./{exe_path.name} {nml_path.name}" in mock_subprocess.commands
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm suddenly wondering if exe_path and nml_path really need to exist? Could this assert be: assert f"./{internal.CABLE_EXE} {internal.CABLE_nml} in mock_subprocess.commands and remove the definition of exe_path and nml_path.

That's a detail, I'm happy to put things in as is. Just mentioning it as I like things as simple as possible when we think of it.

Pull request: #124

Co-authored-by: Claire Carouge <claire.carouge@anu.edu.au>
@SeanBryan51 SeanBryan51 merged commit 79e7fa4 into master Aug 7, 2023
4 checks passed
@SeanBryan51 SeanBryan51 deleted the 123-run-cable-exe-from-task-dir branch August 7, 2023 05:47
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.

CABLE exe should be run from the task directory
2 participants