Sampler and Workflow Refactor #187

nmerket · 2020-10-08T23:03:21Z

Fixes #147.

Pull Request Description

Sampler class refactor. Simplifies things quite a it.

Checklist

Not all may apply

Code changes (must work)
Tests exercising your feature/bug fix (check coverage report on CircleCI build -> Artifacts)
All other unit tests passing
Update validation for project config yaml file changes
Update existing documentation
Run a small batch run to make sure it all works (local is fine, unless an Eagle specific feature)
Add to the changelog_dev.rst file and propose migration text in the pull request

nmerket

@rHorsey It's still kind of rough and doesn't work, but what are your thoughts on the direction of this?

nmerket · 2020-10-12T18:12:05Z

buildstockbatch/base.py

-class SimulationExists(Exception):
-
-    def __init__(self, msg, sim_id, sim_dir):
-        super().__init__(msg)
-        self.sim_id = sim_id
-        self.sim_dir = sim_dir
-
-
-class ValidationError(Exception):
-    pass


These had to be moved to avoid a circular import.

nmerket · 2020-10-12T18:12:53Z

buildstockbatch/base.py

-        else:
-            return os.path.abspath(os.path.join(os.path.dirname(startfile), x))
+    def get_sampler_class(sampler_name):
+        sampler_class_name = ''.join(x.capitalize() for x in sampler_name.strip().split('_')) + 'Sampler'


residential_quota ➡️ ResidentialQuotaSampler

nmerket · 2020-10-12T18:13:31Z

buildstockbatch/base.py

+        Sampler = self.get_sampler_class(self.cfg['sampler']['type'])
+        self.sampler = Sampler(self, **self.cfg['sampler'].get('args', {}))


Much simpler than the logic we used to have. Each sampler will then define its own arguments and have validation for it.

nmerket · 2020-10-12T18:14:06Z

buildstockbatch/base.py

-            key, value = logic.split('|')
-            return df[key] == value
-
-    def downselect(self):


Moved this into its own sampler class.

nmerket · 2020-10-12T18:14:48Z

buildstockbatch/base.py

+    def validate_sampler(project_file):
+        cfg = BuildStockBatchBase.get_project_configuration(project_file)
+        sampler_name = cfg['sampler']['type']
+        try:
+            Sampler = BuildStockBatchBase.get_sampler_class(sampler_name)
+        except AttributeError:
+            raise ValidationError(f'Sampler class `{sampler_name}` is not available.')
+        args = cfg['sampler']['args']
+        return Sampler.validate_args(project_file, **args)


Each sampler defines its own validation, which gets called as part of the validation step.

nmerket · 2020-10-12T18:16:10Z

buildstockbatch/eagle.py

@@ -47,6 +46,8 @@ def get_bool_env_var(varname):

 class EagleBatch(BuildStockBatchBase):

+    CONTAINER_RUNTIME = ContainerRuntime.SINGULARITY


Each Batch class tells has this enum on it to identify which container runtime it uses. The samplers then read this and call the appropriate method. This allows us to not have separate docker and singularity classes.

nmerket · 2020-10-12T18:18:03Z

buildstockbatch/sampler/base.py

-        self.cfg = cfg
-        self.buildstock_dir = buildstock_dir
-        self.project_dir = project_dir
+        self.parent = weakref.ref(parent)  # This removes circular references and allows garbage collection to work.


I pass the parent object to here and use python's weakref to avoid problems with circular references and garbage collection. This allows the child object to read stuff off the parent, such as which container runtime it uses.

nmerket · 2020-10-12T18:20:01Z

buildstockbatch/sampler/downselect.py

+logger = logging.getLogger(__name__)
+
+
+class DownselectSamplerBase(BuildStockSampler):


Subclass from this class to do downselecting for a particular sampler, as long as it takes the n_datapoints arg. Then you specify the downselect version of the sampler if that's what you want.

nmerket · 2020-10-12T18:21:08Z

buildstockbatch/sampler/residential_quota.py

+class ResidentialQuotaDownselectSampler(DownselectSamplerBase):
+    SUB_SAMPLER_CLASS = ResidentialQuotaSampler


Here's one of the downselect versions of a sampler class.

…efactor

nmerket · 2021-01-22T17:04:08Z

@ejhw @rHorsey @joseph-robertson This represents a pretty major change to the configuration files set up. I'm pretty satisfied that's it's working at this point. I'd appreciate it if you guys read over the docs changes and let me know if it communicates it well enough. You can see the generated docs from CicleCI by clicking on the CircleCI check, Artifacts tab, then docs/index.html.

@rHorsey I have not tried this with ComStock. I think it should work. There are gaps in the commercial sobol sampler documentation and tests. If you don't have time to review now we can just merge it and do bugfixes later to get ComStock working again.

afontani · 2021-01-22T18:06:07Z

@nmerket: It makes sense to put the n_datapoints argument in with the sampling arguments. I have been out of the loop on this PR, but if we wanted to add another sampling method does this restructure make that easier?

nmerket · 2021-01-25T15:43:05Z

@afontani, yes it does make it easier to add another sampler. It separates the sampler code into a class that provides a specific interface. If you want to make a new sampler, you just need to create a new class that does sampling according to whatever method you want as long as it matches that interface.

The same is true of workflow generators. This will help with allowing for workflow generators for HPXML and eventually Home Energy Score.

nmerket · 2021-01-25T18:58:37Z

Update yamls:

Local
Eagle
AWS.

…efactor

nmerket · 2021-01-28T18:12:52Z

@ejhw Are we still maintaining project_resstock_national_upgrades.yml?

nmerket · 2021-01-28T18:54:39Z

@ejhw Heads up, I updated project_resstock_national_upgrades.yml to use the new schema version, but it doesn't work because the upgrade options don't match what's in resstock anymore.

ejhw · 2021-01-29T16:47:09Z

@nmerket Thanks for the heads up. Yes, we should still maintain it; though I'd like to move it to /resstock. Do you think that makes sense?

docs/changelog/migration_0_20.rst

buildstockbatch/schemas/v0.3.yaml

project_resstock_national.yml

aws_demo_project.yml

project_resstock_national.yml

project_resstock_national_upgrades.yml

buildstockbatch/test/test_inputs/enforce-validate-measures-bad-2.yml

aws_demo_project.yml

ejhw · 2021-02-09T19:36:16Z

@joseph-robertson Can you approve? Looks like @nmerket resolved your requested changes.

refactoring sampler classes

9e78479

nmerket requested a review from rHorsey October 8, 2020 23:16

passing args to samplers, making downselect a sampler

d53d81d

nmerket commented Oct 12, 2020

View reviewed changes

nmerket added 15 commits October 26, 2020 15:30

mostly fixing validation

f54e575

Merge remote-tracking branch 'origin/develop' into sampler_workflow_r…

eba42ae

…efactor

minor formatting change in tests

674bd94

rearranging workflow generator

91183bf

fixing style issues

89ff745

saving my place after a long absence

0ab2afc

Merge remote-tracking branch 'origin/develop' into sampler_workflow_r…

40a64dc

…efactor

Merge remote-tracking branch 'origin/develop' into sampler_workflow_r…

a093f8a

…efactor

passing n_datapoints into create_osw

19c909e

Merge remote-tracking branch 'origin/develop' into sampler_workflow_r…

a491894

…efactor

fixing workflow generator tests

5ed38ca

fixing tests

beabfd0

changes to project_resstock_national.yml for testing

03f3b6f

fixing eagle errors

29ee6a2

fixing testing mock errors

b68a1b1

nmerket mentioned this pull request Jan 14, 2021

Fix: racially insensitive language #205

Merged

7 tasks

documentation, migration guide, and changelog

331a251

nmerket requested a review from ejhw January 22, 2021 16:58

nmerket marked this pull request as ready for review January 22, 2021 16:58

Merge remote-tracking branch 'origin/develop' into sampler_workflow_r…

d3f6d10

…efactor

nmerket requested a review from joseph-robertson January 22, 2021 17:00

Merge remote-tracking branch 'origin/develop' into sampler_workflow_r…

e4a5349

…efactor

cleaning up a few more OpenStudio-BuildStock to resstock references

2f12c9e

updating example project files

7a1232b

joseph-robertson requested changes Feb 1, 2021

View reviewed changes

docs/changelog/migration_0_20.rst Outdated Show resolved Hide resolved

nmerket added 2 commits February 8, 2021 11:29

adding sample_file key

639cfbf

pinning to pyarrow<3

3b44c8f

ejhw requested changes Feb 8, 2021

View reviewed changes

ejhw reviewed Feb 8, 2021

View reviewed changes

aws_demo_project.yml Outdated Show resolved Hide resolved

nmerket and others added 3 commits February 8, 2021 15:00

yaml file updates for Eric

a993964

Comments on intentional bad inputs

9fa36be

Reorder downselect args

c671702

ejhw self-requested a review February 9, 2021 00:53

ejhw approved these changes Feb 9, 2021

View reviewed changes

joseph-robertson self-requested a review February 9, 2021 19:39

joseph-robertson approved these changes Feb 9, 2021

View reviewed changes

nmerket merged commit 86067e7 into develop Feb 10, 2021

nmerket deleted the sampler_workflow_refactor branch February 10, 2021 15:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sampler and Workflow Refactor #187

Sampler and Workflow Refactor #187

nmerket commented Oct 8, 2020 •

edited

nmerket left a comment

nmerket Oct 12, 2020

nmerket Oct 12, 2020

nmerket Oct 12, 2020

nmerket Oct 12, 2020

nmerket Oct 12, 2020

nmerket Oct 12, 2020

nmerket Oct 12, 2020 •

edited

nmerket Oct 12, 2020

nmerket Oct 12, 2020

nmerket commented Jan 22, 2021

afontani commented Jan 22, 2021

nmerket commented Jan 25, 2021

nmerket commented Jan 25, 2021 •

edited

nmerket commented Jan 28, 2021

nmerket commented Jan 28, 2021

ejhw commented Jan 29, 2021 •

edited

ejhw commented Feb 9, 2021

		Sampler = self.get_sampler_class(self.cfg['sampler']['type'])
		self.sampler = Sampler(self, **self.cfg['sampler'].get('args', {}))

		@@ -47,6 +46,8 @@ def get_bool_env_var(varname):

		class EagleBatch(BuildStockBatchBase):

		CONTAINER_RUNTIME = ContainerRuntime.SINGULARITY

		logger = logging.getLogger(__name__)


		class DownselectSamplerBase(BuildStockSampler):

		class ResidentialQuotaDownselectSampler(DownselectSamplerBase):
		SUB_SAMPLER_CLASS = ResidentialQuotaSampler

Sampler and Workflow Refactor #187

Sampler and Workflow Refactor #187

Conversation

nmerket commented Oct 8, 2020 • edited

Pull Request Description

Checklist

nmerket left a 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

nmerket Oct 12, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nmerket commented Jan 22, 2021

afontani commented Jan 22, 2021

nmerket commented Jan 25, 2021

nmerket commented Jan 25, 2021 • edited

nmerket commented Jan 28, 2021

nmerket commented Jan 28, 2021

ejhw commented Jan 29, 2021 • edited

ejhw commented Feb 9, 2021

nmerket commented Oct 8, 2020 •

edited

nmerket Oct 12, 2020 •

edited

nmerket commented Jan 25, 2021 •

edited

ejhw commented Jan 29, 2021 •

edited