Fix #1130 - Can't run resstock in parallel #1131

jmarrec · 2023-09-05T12:54:30Z

Pull Request Description

Fix Can't run resstock in parallel #1130

I know the BuildExistingModel is in the OpenStudio-HPXML repo and changes should probably be made there, but I would like to make sure this has a chance of being included in ResStock before I make the changes over there as well.

Having the ability to run ResStock in parallel is something I actively need.

Running 125 "baseline" simulations on my 8 CPU / 16 cores machine:

Serially: 40 minutes
Parallel, with the changes here: 4min30 seconds.

I ran 3125 simulations, in paralle, in 2h30 minutes. And I plan to run a lot more.
This would have taken somewhere between 17 and 24 hours serially, rendering it completely unpractical

I'd very much like if this could be upstreamed in ResStock, as it may help other people and would reduce the burden of keeping my fork up to date.

Checklist

Not all may apply:

Tests (and test files) have been updated
Documentation has been updated
- If related to resstock-estimation, checklist includes data dictionary, source report, options_lookup.
- If changes to project_testing tsvs, checklist includes yml_precomputed, yml_precomputed_outdated, yml_precomputed_weight
Changelog has been updated
openstudio tasks.rb update_measures has been run
No unexpected regression test changes on CI (checked comparison artifacts)

jmarrec · 2023-09-05T12:56:53Z

workflow/run_analysis.rb

+def checksum_dir_content(directory_path)
+  files = Dir.glob('**/*', base: directory_path).select { |fn| File.file?(fn) }
+  dir_checksum = Zlib::crc32(files.map { |rel_path|
+    [
+      rel_path,
+      File.mtime(File.join(directory_path, rel_path)), # mtime is affected by the copy, but we passed preserve = true
+      File.size(File.join(directory_path, rel_path))
+    ]
+  }.to_s)
+  return dir_checksum
+end
+
+def create_lib_folder(lib_dir, resources_dir, housing_characteristics_dir, debug: false)
+  redo_needed = true
+  if File.directory?(lib_dir)
+    lib_resources_dir = File.join(lib_dir, File.basename(resources_dir))
+    resource_matches = checksum_dir_content(resources_dir) == checksum_dir_content(lib_resources_dir)
+    if resource_matches
+      lib_housing_characteristics_dir = File.join(lib_dir, File.basename(housing_characteristics_dir))
+      housing_matches = checksum_dir_content(housing_characteristics_dir) == checksum_dir_content(lib_housing_characteristics_dir)
+      if housing_matches
+        redo_needed = false
+      elsif debug
+        puts "Housing directory is outdated: #{lib_housing_characteristics_dir}"
+      end
+    elsif debug
+      puts "Resources directory is outdated: #{lib_resources_dir}"
+    end
+  elsif debug
+    puts "Creating lib_dir"
+  end
+
+  if !redo_needed
+    if debug
+      puts "Lib folder is up to date"
+    end
+    return
+  end
  FileUtils.rm_rf(lib_dir)
  Dir.mkdir(lib_dir)
-  FileUtils.cp_r(resources_dir, lib_dir)
-  FileUtils.cp_r(housing_characteristics_dir, lib_dir)
+
+  # Preserve object’s group, user and **modification time** on copying
+  FileUtils.cp_r(resources_dir, lib_dir, preserve: true)
+  FileUtils.cp_r(housing_characteristics_dir, lib_dir, preserve: true)


Detect if the resstock/lib folder has to be created in a "smart" way:

If it's missing...

If it's there, but the content is outdated

To check whether that's outdated, I compute a checksum for all [filename, file modification time, file size].

This is much faster than using OpenStudio::checksum (implementation given below for posterity)

def checksum_dir_content_os(directory_path) files = Dir.glob(File.join(directory_path, '**/*')).select { |fn| File.file?(fn) } dir_checksum = OpenStudio::checksum(files.map { |abs_path| OpenStudio::checksum(OpenStudio::toPath(abs_path)) }.to_s) return dir_checksum end

Note that one has to pass --debug to run_analysis.rb to avoid wiping the lib folder at the end of the run as well.

jmarrec · 2023-09-05T12:58:57Z

measures/BuildExistingModel/measure.rb

+    arg = OpenStudio::Ruleset::OSArgument.makeStringArgument('buildstock_csv_path', true)
+    arg.setDisplayName('Buildstock CSV File Path')
+    arg.setDescription('Absolute/relative path of the buildstock CSV file, relative is compared to the lib/housing_characteristics_dir.')
+    args << arg


New BuildExistingModel argument: buildstock_csv_path.

Maybe that could be made optional and still default to lib/housing_characteristics/buildstock.csv by setting the default value to buildstock.csv.

jmarrec · 2023-09-05T12:59:27Z

measures/BuildExistingModel/measure.rb

+    buildstock_csv_path = args['buildstock_csv_path']
+    unless (Pathname.new buildstock_csv_path).absolute?
+      buildstock_csv_path = File.absolute_path(File.join(characteristics_dir, buildstock_csv_path)) # Should have been generated by the Worker Initialization Script (run_sampling.rb) or provided by the project
+    end


If buildstock_csv_path is absolute, use that. Otherwise get it relative to the lib/housing_characteristics dir

jmarrec · 2023-09-05T12:59:47Z

workflow/run_analysis.rb

+    # TODO: this should write directly to the results_dir...
+    # run_sampling_lib::write_csv should  not take a relative path relative to
+    # the resources/run_sampling_lib.rb but an absolute path
    create_buildstock_csv(project_directory, n_datapoints, outfile)


jmarrec · 2023-09-05T13:00:21Z

workflow/run_analysis.rb

+    buildstock_csv_path = File.join(results_dir, "buildstock.csv")
+    FileUtils.cp(src, buildstock_csv_path)


If sampled, after copying it, use the copy in the results_dir instead of the lib/housing_characteristics/buildstock.csv

jmarrec · 2023-09-05T13:01:06Z

workflow/run_analysis.rb

+    # If buildingstock_csv is absolute, just use that
+    # If relative: relative to yml
+    buildstock_csv_path = cfg['sampler']['args']['sample_file']
+    unless (Pathname.new buildstock_csv_path).absolute?
+      buildstock_csv_path = File.expand_path(File.join(File.dirname(yml), buildstock_csv_path))
+    end

-    buildstock_csv = CSV.read(des, headers: true)
+    buildstock_csv = CSV.read(buildstock_csv_path, headers: true)


If precomputed, use the sample > args > sample_file.

If absolute, all good

If not, find it relative to the workflow YAML, as per before.

jmarrec · 2023-09-05T13:01:24Z

workflow/run_analysis.rb

@@ -116,6 +123,7 @@ def run_workflow(yml, n_threads, measures_only, debug_arg, overwrite, building_i
    }

    bld_exist_model_args = {
+      'buildstock_csv_path': buildstock_csv_path,


Pass that buildstock_csv_path argument to BuildExistingModel

joseph-robertson · 2023-09-07T14:40:42Z

@jmarrec So the basic idea here is making the path of the buildstock.csv file variable (instead of fixed at lib/housing_characteristics) so that you can use run_analysis.rb in parallel?

Couple of questions.
(1) How can we get this branch running on CI?
(2) If we did, I'd assume we'd see problems with the integration-tests job that uses buildstockbatch (where you haven't yet updated its workflow generator for the BuildExistingModel update). Is there a way to make the new buildstock_csv_path argument optional (and default to the original lib/housing_characteristics) so we wouldn't need to make any workflow generator changes? Edit: looks like above you are already noting we could do this 👍 .

…f it's already up to date Compute a checksum by checking file name, file modified time, and file size. It checksums match, you are guaranteed to have a resstock/lib folder that's fully up to date.

…csv_path instead of expecting it at lib/housing_characteristics/buildstock.csv

jmarrec · 2023-09-07T14:53:32Z

@jmarrec So the basic idea here is making the path of the buildstock.csv file variable (instead of fixed at lib/housing_characteristics) so that you can use run_analysis.rb in parallel?

Yes, and don't wipe the lib folder if it's already there and up to date.

Couple of questions.

(1) How can we get this branch running on CI?

Your CI rules seem to protect against running on a branch from a fork... That's a bad setting IMHO. You should set it so workflows coming from a fork don't run automatically but required a maintainer to approve the run.

Anyways, I'm cloning this PR onto my forked repo so it runs there: jmarrec#1

(2) If we did, I'd assume we'd see problems with the integration-tests job that uses buildstockbatch (where you haven't yet updated its workflow generator for the BuildExistingModel update). Is there a way to make the new buildstock_csv_path argument optional (and default to the original lib/housing_characteristics) so we wouldn't need to make any workflow generator changes? Edit: looks like above you are already noting we could do this 👍 .

Yes we can. We'll see if it's necessary.

measures/BuildExistingModel/measure.rb

joseph-robertson · 2024-04-10T21:10:36Z

@jmarrec Should we still consider getting this PR into a state to be merged?

jmarrec · 2024-04-15T07:27:32Z

@joseph-robertson I think the change makes sense, but as a maintainer you're always entitled to reject a pull request as something you don't want to maintain or don't see the point of. Up to you really. (I won't be mad if that's the case, just close it)

The conflicts are trivial to fix and I can resolve them if needed.

joseph-robertson · 2024-04-18T18:06:10Z

Superseded by #1230.

jmarrec commented Sep 5, 2023

View reviewed changes

jmarrec mentioned this pull request Sep 7, 2023

Allow running resstock in Parallel PerceptixAI/resstock#10

Merged

5 tasks

joseph-robertson assigned jmarrec Sep 7, 2023

joseph-robertson added the feature label Sep 7, 2023

jmarrec added 2 commits September 7, 2023 16:45

Parallel resstock: Step one: don't wipe and recreate the Lib folder i…

c89e46d

…f it's already up to date Compute a checksum by checking file name, file modified time, and file size. It checksums match, you are guaranteed to have a resstock/lib folder that's fully up to date.

Parallel resstock: step 2, make BuildExistingModel take a buildstock_…

193a959

…csv_path instead of expecting it at lib/housing_characteristics/buildstock.csv

jmarrec force-pushed the parallel_resstock_develop branch from c346196 to 193a959 Compare September 7, 2023 14:45

jmarrec mentioned this pull request Sep 7, 2023

Parallel resstock develop jmarrec/resstock#1

Open

7 tasks

jmarrec commented Sep 7, 2023

View reviewed changes

measures/BuildExistingModel/measure.rb Outdated Show resolved Hide resolved

Make buildstock_csv_path optional

6239781

joseph-robertson mentioned this pull request Apr 15, 2024

Run run_analysis in parallel #1230

Merged

6 tasks

joseph-robertson closed this Apr 18, 2024

willf mentioned this pull request Feb 12, 2025

Fix #1130 - Can't run resstock in parallel Test-Cloned-Org/resstock#218

Open

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #1130 - Can't run resstock in parallel #1131

Fix #1130 - Can't run resstock in parallel #1131

jmarrec commented Sep 5, 2023 •

edited

Loading

jmarrec Sep 5, 2023

jmarrec Sep 5, 2023

jmarrec Sep 5, 2023

jmarrec Sep 5, 2023

jmarrec Sep 5, 2023

jmarrec Sep 5, 2023

jmarrec Sep 5, 2023

jmarrec Sep 5, 2023

joseph-robertson commented Sep 7, 2023 •

edited

Loading

jmarrec commented Sep 7, 2023

joseph-robertson commented Apr 10, 2024

jmarrec commented Apr 15, 2024

joseph-robertson commented Apr 18, 2024

		buildstock_csv_path = File.join(results_dir, "buildstock.csv")
		FileUtils.cp(src, buildstock_csv_path)

Fix #1130 - Can't run resstock in parallel #1131

Fix #1130 - Can't run resstock in parallel #1131

Conversation

jmarrec commented Sep 5, 2023 • edited Loading

Pull Request Description

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joseph-robertson commented Sep 7, 2023 • edited Loading

jmarrec commented Sep 7, 2023

joseph-robertson commented Apr 10, 2024

jmarrec commented Apr 15, 2024

joseph-robertson commented Apr 18, 2024

jmarrec commented Sep 5, 2023 •

edited

Loading

joseph-robertson commented Sep 7, 2023 •

edited

Loading