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

merge from Qiushi's fork to yewsg/yews: add polarity and focal_mechanism models #18

Merged
merged 275 commits into from
Aug 1, 2020

Conversation

zjzzqs
Copy link
Collaborator

@zjzzqs zjzzqs commented Jul 28, 2020

add polarity and focal_mechanism models

lijunzh and others added 30 commits April 12, 2019 12:34
Protect trademarks and logos
* Fix bug in travis.yml

* add codecov badge

* add python 3.7 to travis.ci

* Add python 3.7 travis image

* add download badge
Add appveyor.yml for windows CI
* add more test to transforms

* replace torch and numpy in module by direct import

* add tests for transform correctness
* init docs by sphinx.

* Update documentation theme to blue

* add doc to README
commit ce4b445
Author: Lijun Zhu <gatechzhu@gmail.com>
Date:   Wed Apr 17 13:12:43 2019 -0400

    yews.transform under cover with 100% coverage.

commit 2cf6108
Author: Lijun Zhu <lijunzh@users.noreply.github.com>
Date:   Tue Apr 16 09:01:48 2019 -0400

    use www subdomain for docs

commit 4c0b060
Author: Lijun Zhu <gatechzhu@gmail.com>
Date:   Wed Apr 17 13:15:07 2019 -0400

    add is_dataset to check dataset-like objects

commit 5a7b0e8
Author: Lijun Zhu <gatechzhu@gmail.com>
Date:   Tue Apr 16 21:46:35 2019 -0400

    Refactorize yews.datasets
commit cbb2b6b
Author: Lijun Zhu <gatechzhu@gmail.com>
Date:   Wed Apr 17 15:04:07 2019 -0400

    rename module to avoid python built-ins

commit e9ebf46
Author: Lijun Zhu <gatechzhu@gmail.com>
Date:   Wed Apr 17 15:27:41 2019 -0400

    yews.files under cover.

commit 744daae
Author: Lijun Zhu <gatechzhu@gmail.com>
Date:   Wed Apr 17 15:04:24 2019 -0400

    yews.datasets.dirs under cover

commit 1f8be24
Author: Lijun Zhu <gatechzhu@gmail.com>
Date:   Wed Apr 17 15:04:07 2019 -0400

    rename module to avoid python built-ins

commit df8c897
Author: Lijun Zhu <gatechzhu@gmail.com>
Date:   Wed Apr 17 13:44:15 2019 -0400

    add test to yews.datasets

commit ce4b445
Author: Lijun Zhu <gatechzhu@gmail.com>
Date:   Wed Apr 17 13:12:43 2019 -0400

    yews.transform under cover with 100% coverage.

commit 2cf6108
Author: Lijun Zhu <lijunzh@users.noreply.github.com>
Date:   Tue Apr 16 09:01:48 2019 -0400

    use www subdomain for docs

commit 4c0b060
Author: Lijun Zhu <gatechzhu@gmail.com>
Date:   Wed Apr 17 13:15:07 2019 -0400

    add is_dataset to check dataset-like objects

commit 5a7b0e8
Author: Lijun Zhu <gatechzhu@gmail.com>
Date:   Tue Apr 16 21:46:35 2019 -0400

    Refactorize yews.datasets
Use both lijunzhu and pytorch channels.
commit efe8105c558319d8145b0033f9c108466ca9ad97
Author: Lijun Zhu <gatechzhu@gmail.com>
Date:   Wed Apr 17 21:57:49 2019 -0400

    automate building process

commit b9b5ba8f86129ce422d0d521e26064c12bbb88e8
Author: Lijun Zhu <gatechzhu@gmail.com>
Date:   Wed Apr 17 21:57:07 2019 -0400

    hide usage of scipy until necessary

commit 9a7c981781ef4f32b346988e493feaf6be02b9dc
Author: Lijun Zhu <gatechzhu@gmail.com>
Date:   Wed Apr 17 20:36:35 2019 -0400

    comply with PyPI rst requirement.
@lijunzh lijunzh self-requested a review July 28, 2020 12:00
Copy link
Member

@lijunzh lijunzh left a comment

Choose a reason for hiding this comment

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

Please take a look at my reviews and change accordingly. Let me know if you have questions.

.travis.yml Outdated
@@ -5,7 +5,10 @@ matrix:
include:
- python: "3.6"
- python: "3.7"
<<<<<<< HEAD
Copy link
Member

Choose a reason for hiding this comment

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

This is a text generated by git when solving conflicts. Needs to be removed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Resolve. Please check it again. Thanks.

README.rst Outdated
@@ -16,15 +16,10 @@ applying deep learning techniques on seismic waveform data.



.. image:: https://travis-ci.org/yewsg/yews.svg?branch=master
:target: https://travis-ci.org/yewsg/yews
Copy link
Member

Choose a reason for hiding this comment

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

Why do we want to remove the badges here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Resolve. Please check it again. Thanks.

@@ -1,2 +1,4 @@
from .cpic import *
from .polarity import *
from .focal_mechanism import *

Copy link
Member

Choose a reason for hiding this comment

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

This is OK now. But we will later want to selectively import model constructors into init.py so that we can the limit usage of model classes into restricted ways that has been tested.


model_urls = {
'cpic_v1': 'https://www.dropbox.com/s/ckb4glf35agi9xa/cpic_v1_wenchuan-bdd92da2.pth?dl=1',
'cpic_v2': 'https://www.dropbox.com/s/kyiuprnn8014fs5/cpic_v2_wenchuan-ee92060a.pth?dl=1'
'cpic_v2': 'https://www.dropbox.com/s/kyiuprnn8014fs5/cpic_v2_wenchuan-ee92060a.pth?dl=1',
'cpic_v3': 'xxxx'
Copy link
Member

Choose a reason for hiding this comment

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

Are you planning to upload a pretrained model for cpic_v3? You can use '' instead of 'xxxx' for place holder.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Resolve. Please check it again. Thanks.

@@ -12,6 +12,7 @@ def get_torch_device():

def model_on_device(model, device):
return torch.nn.DataParallel(model.to(device))
#return torch.nn.DataParallel(model,device_ids=[0]).to("cuda")
Copy link
Member

Choose a reason for hiding this comment

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

I understand that you want to select which GPU to use. However, that is precisely the reason why I passed in device as an argument. You may consider using something below:

device = torch.device("cuda:0" if torch.cuda.is_available() else "cpu")
model_on_device(model, device)

In general, you want to control the selection of GPU in your main script instead of inside the trainer class. Thus, it is better to be passed in as an argument.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Resolve. Please check it again. Thanks.

<<<<<<< HEAD
from scipy import signal
=======
>>>>>>> upstream/master

Copy link
Member

Choose a reason for hiding this comment

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

Same here. Fix the git artifects.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Resolve. Please check it again. Thanks.

b, a = signal.butter(order, [low, high], btype='bandpass')
wav = signal.filtfilt(b, a, wav, axis=-1, padtype=None, padlen=None, irlen=None)

return wav
Copy link
Member

Choose a reason for hiding this comment

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

Please consider adding unit tests to the transforms added here. You only needs some basic cases to cover the straightforward usages.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Resolve. Please check it again. Thanks.

@@ -0,0 +1,331 @@
import torch.nn as nn
Copy link
Member

Choose a reason for hiding this comment

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

Did not check the correctness here. Since you are working on the model, you konw better than me.

trainer = Trainer(model, CrossEntropyLoss(), lr=0.1)
trainer.validate(val_loader, print_freq=100)

print("Now: end : " + str(datetime.datetime.now()))
Copy link
Member

Choose a reason for hiding this comment

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

I am in the process of revamping the underlying training tools (multi-process up to 8 GPUs and a logging system). We may need to change this later. In the meantime, feel free to change it as long as it does not error out.

@zjzzqs zjzzqs requested a review from lijunzh August 1, 2020 06:57
@lijunzh lijunzh merged commit 730a51e into yewsg:master Aug 1, 2020
@lijunzh
Copy link
Member

lijunzh commented Aug 1, 2020

@zjzzqs I've merged your changes. There are still some places that we can discuss and improve. But, we should do it in another PR next time.

lijunzh added a commit that referenced this pull request Aug 3, 2020
* master:
  merge from Qiushi's fork to yewsg/yews: add polarity and focal_mechanism models (#18)
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.

3 participants