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

Decouple job and mixins #624

Merged
merged 57 commits into from Jan 11, 2012
Merged

Conversation

larsbutler
Copy link
Contributor

This branch addresses: https://bugs.launchpad.net/openquake/+bug/908148

The major change here is that calculator Mixin classes must be instantiated with a Job object. The Job and the Mixin are no longer mashed together at runtime into a Frankenobject; instead, we use a silly little pattern called 'composition'.

There is still a bit of a mess here; the mixins still exist and some things are not quite how we would like them to be. This branch, however, is a big step towards our goal.

For a few of the next steps in Operation: Destroy the Mixins, see:
https://bugs.launchpad.net/openquake/+bug/909663
https://bugs.launchpad.net/openquake/+bug/907243

https://github.com/gem/openquake/pull/622 and https://github.com/gem/openquake/pull/623 should land first.

@kpanic
Copy link

kpanic commented Jan 4, 2012

pylint pick

Running pylint on larsbutler/decouple-job-and-mixins..

!! larsbutler/decouple-job-and-mixins adds 30 pylint issues..

``--- /tmp/pl.a 2012-01-04 17:39:02.000000000 +0100
+++ /tmp/pl.b 2012-01-04 17:39:15.000000000 +0100
@@ -2,0 +3 @@
+openquake/engine.py: [W0212, launch] Access to a protected member _record_initial_stats of a client class
@@ -9,0 +11,7 @@
+openquake/hazard/calc.py: [C0111] Missing docstring
+openquake/hazard/calc.py: [C0111, _load_calcs] Missing docstring
+openquake/hazard/opensha.py: [W0404] Reimport 'kvs' (imported line 38)
+openquake/hazard/opensha.py: [W0404] Reimport 'BasePSHAMixin' (imported line 45)
+openquake/hazard/opensha.py: [W0404] Reimport 'get_iml_list' (imported line 45)
+openquake/hazard/opensha.py: [W0404, compute_ground_motion_fields] Reimport 'CALCULATORS' (imported line 28)
+openquake/hazard/opensha.py: [W0404, compute_hazard_curve] Reimport 'CALCULATORS' (imported line 28)
@@ -11,0 +20,22 @@
+openquake/risk/calc.py: [C0111] Missing docstring
+openquake/risk/calc.py: [C0111, _load_calcs] Missing docstring
+openquake/risk/job/general.py: [W0404, compute_risk] Reimport 'CALCULATORS' (imported line 23)
+openquake/nrml/init.py: [R0801] Similar lines in 2 files
+==openquake.risk.job.classical_psha
+==openquake.risk.job.probabilistic

  •                for loss_poe in general.conditional_loss_poes(
    
  •                    self.job_profile.params):
    
  •                    general.compute_conditional_loss(
    
  •                            self.job_profile.job_id, point.column,
    
  •                            point.row, loss_curve, asset, loss_poe)
    
    +openquake/nrml/init.py: [R0801] Similar lines in 2 files
    +==openquake.risk.job.classical_psha
    +==openquake.risk.job.probabilistic
  •    bcr_block_key = kvs.tokens.bcr_block_key(self.job_profile.job_id,
    
  •                                             block_id)
    
  •    kvs.set_value_json_encoded(bcr_block_key, result)
    
  •    LOGGER.debug('bcr result for block %s: %r', block_id, result)
    
  •    return True
    
    +``

pep8 pick

Running pep8 on larsbutler/decouple-job-and-mixins..

openquake/output/template.py:23:80: E501 line too long (103 characters) openquake/output/template.py:89:80: E501 line too long (103 characters)

@kpanic
Copy link

kpanic commented Jan 4, 2012

Changes have to be done accordingly to celeryconfig.py

kpanic@hpinux:~/src/openquake-kpanic (bb)$ celeryd Traceback (most recent call last): File "/usr/local/bin/celeryd", line 9, in <module> load_entry_point('celery==2.3.1', 'console_scripts', 'celeryd')() File "/usr/lib/pymodules/python2.7/celery/bin/celeryd.py", line 187, in main worker.execute_from_commandline() File "/usr/lib/pymodules/python2.7/celery/bin/base.py", line 72, in execute_from_commandline return self.handle_argv(prog_name, argv[1:]) File "/usr/lib/pymodules/python2.7/celery/bin/base.py", line 100, in handle_argv return self.run(*args, **vars(options)) File "/usr/lib/pymodules/python2.7/celery/bin/celeryd.py", line 96, in run return self.app.Worker(**kwargs).run() File "/usr/lib/pymodules/python2.7/celery/apps/worker.py", line 124, in run self.worker_init() File "/usr/lib/pymodules/python2.7/celery/apps/worker.py", line 183, in worker_init self.loader.init_worker() File "/usr/lib/pymodules/python2.7/celery/loaders/base.py", line 84, in init_worker self.on_worker_init() File "/usr/lib/pymodules/python2.7/celery/loaders/default.py", line 54, in on_worker_init self.import_default_modules() File "/usr/lib/pymodules/python2.7/celery/loaders/base.py", line 79, in import_default_modules for module in imports | self.builtin_modules] File "/usr/lib/pymodules/python2.7/celery/loaders/base.py", line 67, in import_task_module return self.import_from_cwd(module) File "/usr/lib/pymodules/python2.7/celery/loaders/base.py", line 74, in import_from_cwd self.import_module if imp is None else imp) File "/usr/lib/pymodules/python2.7/celery/utils/__init__.py", line 407, in import_from_cwd return imp(module) File "/usr/lib/pymodules/python2.7/celery/loaders/base.py", line 70, in import_module return importlib.import_module(module) File "/usr/lib/python2.7/importlib/__init__.py", line 37, in import_module __import__(name) ImportError: No module named tasks

@larsbutler
Copy link
Contributor Author

I'm going to ignore the pep8 warnings in openquake/output/template.py; it's just a bunch of text for an html template (it's not code). Is that okay with you?

@kpanic
Copy link

kpanic commented Jan 5, 2012

yep

@larsbutler
Copy link
Contributor Author

Cool.

Regarding the celeryd import issue, I'm not able to reproduce this error.. but I see the problem. Celery is importing stuff that isn't there anymore so I will update the config. I'll submit an update in just a second.

@kpanic
Copy link

kpanic commented Jan 5, 2012

Lars, maybe you have an old celeryconfig.pyc or tasks.pyc ?

try:

find -name \*pyc|xargs -i rm {}

@al-maisan
Copy link

More pylint fun:
openquake/logs.py: [E0202, AMQPHandler.emit] An attribute inherited from SupervisorLogMessageConsumer hide this method
openquake/output/init.py: [C0111] Missing docstring

@al-maisan
Copy link

pep8:

openquake/output/template.py:23:80: E501 line too long (103 characters)
openquake/output/template.py:89:80: E501 line too long (103 characters)

@al-maisan
Copy link

pyflakes:

tests/hazard_unittest.py:40: 'mixins' imported but unused
tests/hazard_unittest.py:47: 'openquake' imported but unused
tests/input_risk_unittest.py:24: 'Job' imported but unused
tests/input_risk_unittest.py:25: 'from openquake.output.hazard import *' used; unable to detect undefined names
tests/input_risk_unittest.py:30: 'Region' imported but unused
tests/job_unittest.py:38: 'Mixin' imported but unused
tests/job_unittest.py:41: 'general' imported but unused
tests/job_unittest.py:42: 'ProbabilisticEventMixin' imported but unused
tests/job_unittest.py:43: 'ClassicalPSHABasedMixin' imported but unused
tests/risk_job_unittest.py:25: 'job' imported but unused
tests/risk_job_unittest.py:29: 'Mixin' imported but unused
tests/utils/helpers.py:31: 'subprocess' imported but unused

@al-maisan
Copy link

Also:

openquake/job/__init__.py:23: 'urlparse' imported but unused
openquake/job/__init__.py:34: 'logs' imported but unused
openquake/job/__init__.py:43: 'Mixin' imported but unused

@al-maisan
Copy link

Typo in openquake/hazard/init.py

s/functionaity/functionality/

@al-maisan
Copy link

LGTM

larsbutler added a commit that referenced this pull request Jan 11, 2012
[r=al-maisan] [f=908148]
Decouple job and mixins
@larsbutler larsbutler merged commit a77266b into gem:master Jan 11, 2012
daniviga pushed a commit that referenced this pull request Jun 7, 2017
Implements the Shahjouei & Pezeshk (2016) GMM
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.

None yet

3 participants