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

Develop code review #1

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Develop code review #1

wants to merge 4 commits into from

Conversation

nocliper
Copy link
Owner

@nocliper nocliper commented Nov 2, 2023

No description provided.

@nocliper nocliper added the good first issue Good for newcomers label Nov 2, 2023
@nocliper nocliper requested a review from kafanovd November 2, 2023 13:14
modules/L1L2.py Outdated

import numpy as np
from scipy.sparse import diags
from sklearn.linear_model import ElasticNet
Copy link
Collaborator

Choose a reason for hiding this comment

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

импорты в начало

modules/L1L2.py Outdated
from sklearn.linear_model import ElasticNet

# set up grid points (# = Nz)
h = np.log(bound[1]/bound[0])/(Nz - 1) # equally spaced on logscale
Copy link
Collaborator

Choose a reason for hiding this comment

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

пробелы

modules/L1L2.py Outdated

# construct C matrix from [1]
s_mesh, t_mesh = np.meshgrid(s, t)
C = np.exp(-t_mesh*s_mesh)
Copy link
Collaborator

Choose a reason for hiding this comment

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

пробелы

modules/L1L2.py Outdated
C *= h

alpha = alpha1 + alpha2
l1_ratio = alpha1/alpha
Copy link
Collaborator

Choose a reason for hiding this comment

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

пробелы

modules/L1L2.py Outdated

# set up grid points (# = Nz)
h = np.log(bound[1]/bound[0])/(Nz - 1) # equally spaced on logscale
s = bound[0]*np.exp(np.arange(Nz)*h) # z (Nz by 1)
Copy link
Collaborator

Choose a reason for hiding this comment

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

можно переменные обозвать терминами, чтобы более читабельно было

Solution of argmin_x || Ax - b ||_2
"""

from scipy.optimize import nnls
Copy link
Collaborator

Choose a reason for hiding this comment

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

импорт в начало

C = np.exp(-t_mesh*z_mesh)
C[:, 0] /= 2.
C[:, -1] /= 2.
C *= h
Copy link
Collaborator

Choose a reason for hiding this comment

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

```
h = np.log(bound[1]/bound[0])/(Nz - 1)      # equally spaced on logscale
z = bound[0]*np.exp(np.arange(Nz)*h)        # z (Nz by 1)

z_mesh, t_mesh = np.meshgrid(z, t)
C = np.exp(-t_mesh*z_mesh)       
C[:, 0] /= 2.
C[:, -1] /= 2.
C *= h```

У тебя этот кусок используется много раз. Можно написать метод отдельный для него, чтобы не дублировать код

modules/demo.py Outdated
from hp import hp
from read_file import read_file
from regopt import regopt
from interface import interface
Copy link
Collaborator

Choose a reason for hiding this comment

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

в начало

modules/demo.py Outdated

Bounds = 10.0**np.asarray(Bounds)

t, C, T = read_file(interface.path, dt, proc = True)# time, transients, temperatures
Copy link
Collaborator

Choose a reason for hiding this comment

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

коммент и пробелы убрать для proc

self.path = os.getcwd()
self._update_files()

def _update_files(self):
Copy link
Collaborator

Choose a reason for hiding this comment

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

здесь тоже для всех методов типы и анотации

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants