Skip to content

Commit

Permalink
Merge pull request #420 from slayoo/dt_coal
Browse files Browse the repository at this point in the history
adding test that checks for TODO annotations; updating TODO annotations
  • Loading branch information
slayoo committed Feb 16, 2021
2 parents 42d46c6 + cfff23e commit 244a8de
Show file tree
Hide file tree
Showing 32 changed files with 111 additions and 49 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ jobs:
- run: |
pip install -r requirements.txt
# TODO (https://github.com/numba/numba/issues/6350#issuecomment-728174860)
# TODO #184 (https://github.com/numba/numba/issues/6350#issuecomment-728174860)
- if: matrix.platform == 'ubuntu-latest'
env:
NUMBA_THREADING_LAYER: omp
Expand Down
4 changes: 2 additions & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ jobs:
# https://github.com/pypa/virtualenv/pull/1064/files
- sed -i 's/"rU"/"r"/g' /home/travis/virtualenv/python3.8.0/lib/python3.8/site.py
- cat /home/travis/virtualenv/python3.8.0/lib/python3.8/site.py
- export PY_FLAGS="" # TODO numba 0.52 triggers deprecation warnings with newest numpy
- export PY_FLAGS="" # TODO #408: numba 0.52 triggers deprecation warnings with newest numpy
- name: "Python 3.7 on Linux numba::parallel=False"
before_script:
- sed -i s/parallel=True/parallel=False/g PySDM/backends/numba/conf.py
Expand Down Expand Up @@ -57,7 +57,7 @@ script:
- NUMBA_DISABLE_JIT=1 $PYTEST PySDM_tests/unit_tests --cov-report term --cov=PySDM
- $PYTEST
- |
python$PY_SFX -m ipykernel install --user # TODO: https://github.com/ipython/ipykernel/issues/560
python$PY_SFX -m ipykernel install --user
for i in PySDM_examples/*/demo.ipynb; do
$NBCONVERT $i > $i.md.repo;
$NBCONVERT --execute --ExecutePreprocessor.timeout=1800 $i || exit 1;
Expand Down
2 changes: 1 addition & 1 deletion PySDM/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,4 @@

from .builder import Builder

__version__ = "TODO"
__version__ = "TODO #415"
4 changes: 2 additions & 2 deletions PySDM/backends/numba/impl/_algorithmic_methods.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ def adaptive_sdm_gamma_body(gamma, idx, length, n, cell_id, dt_left, dt, dt_rang
dt_todo = np.empty_like(dt_left)
for cid in prange(len(dt_todo)):
dt_todo[cid] = min(dt_left[cid], dt_range[1])
for i in range(length // 2): # TODO: #401
for i in range(length // 2): # TODO #401
if gamma[i] == 0:
continue
j, k = pair_indices(i, idx, is_first_in_pair)
Expand Down Expand Up @@ -173,7 +173,7 @@ def compute_gamma_body(gamma, rand, idx, length, n, cell_id,

j, k = pair_indices(i, idx, is_first_in_pair)
prop = n[j] // n[k]
g = min(int(gamma[i]), prop) # TODO: test asserting that min is not needed with adaptivity
g = min(int(gamma[i]), prop) # TODO #416: test asserting that min is not needed with adaptivity
cid = cell_id[j]
collision_rate[cid] += g * n[k]
collision_rate_deficit[cid] += (int(gamma[i]) - g) * n[k]
Expand Down
1 change: 0 additions & 1 deletion PySDM/backends/numba/storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,6 @@ def product(self, multiplicand, multiplier):
impl.multiply_out_of_place(self.data, multiplicand.data, multiplier)
return self

# TODO #352 rename (different logic than np.ravel())
def ravel(self, other):
if isinstance(other, Storage):
self.data[:] = other.data.ravel()
Expand Down
5 changes: 2 additions & 3 deletions PySDM/backends/thrustRTC/fakeThrustRTC/fakeThrustRTC.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ def __init__(self, ndarray):
self.range = lambda start, stop: FakeThrustRTC.DVRange(self.ndarray[start: stop])
self.to_host = lambda: np.copy(self.ndarray)

# TODO: should not be needed
def __setitem__(self, key, value):
if isinstance(value, FakeThrustRTC.Number):
value = value.ndarray
Expand Down Expand Up @@ -102,7 +101,7 @@ def Find(vector, value):

@staticmethod
def device_vector(elem_cls, size):
if elem_cls == 'double' or elem_cls == 'float': # TODO: distinguish np.float32 and np.float64?
if elem_cls == 'double' or elem_cls == 'float': # TODO #405
dtype = np.float64
elif elem_cls == 'int64_t':
dtype = np.int64
Expand Down Expand Up @@ -185,4 +184,4 @@ def Wait():
@staticmethod
def Sort_By_Key(keys, values):
values.ndarray[:] = values.ndarray[np.argsort(keys.ndarray)]
# TODO #328 Thrust sorts keys as well
# TODO #328 Thrust sorts keys as well
7 changes: 2 additions & 5 deletions PySDM/backends/thrustRTC/impl/_algorithmic_methods.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ def adaptive_sdm_end(dt_left, cell_start):
@staticmethod
@nice_thrust(**NICE_THRUST_FLAGS)
def adaptive_sdm_gamma(gamma, n, cell_id, dt_left, dt, dt_range, is_first_in_pair, stats_n_substep, stats_dt_min):
# TODO: implement stats_dt_min
# TODO #406 implement stats_dt_min
dt_todo = trtc.device_vector('float', len(dt_left))
d_dt_max = PrecisionResolver.get_floating_point(dt_range[1])
d_dt = PrecisionResolver.get_floating_point(dt)
Expand Down Expand Up @@ -430,10 +430,7 @@ def remove_zero_n_or_flagged(data, idx, length) -> int:
@staticmethod
@nice_thrust(**NICE_THRUST_FLAGS)
def _sort_by_cell_id_and_update_cell_start(cell_id, cell_idx, cell_start, idx, length):
# TODO #69
# if length > 0:
# max_cell_id = max(cell_id.to_ndarray())
# assert max_cell_id == 0
# TODO #330
trtc.Fill(cell_start.data, trtc.DVInt64(length))
AlgorithmicMethods.___sort_by_cell_id_and_update_cell_start_body.launch_n(length - 1,
[cell_id.data, cell_start.data,
Expand Down
4 changes: 2 additions & 2 deletions PySDM/dynamics/displacement.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,6 @@ def update_cell_origin(self, cell_origin, position_in_cell):
position_in_cell -= floor_of_position

def boundary_condition(self, cell_origin):
# TODO #346 hardcoded periodic
# TODO #346 droplets above the mesh
# TODO #414 hardcoded periodic
# TODO #414 droplets above the mesh
cell_origin %= self.grid
2 changes: 1 addition & 1 deletion PySDM/dynamics/eulerian_advection/mpdata.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ def wait(self):
self.thread.join()

def step(self):
try: # TODO: move this to within PyMPDATA
try: # TODO #417: move this to within PyMPDATA
for mpdata in self.mpdatas.values():
mpdata.advance(1)
except NumbaExperimentalFeatureWarning:
Expand Down
2 changes: 1 addition & 1 deletion PySDM/environments/kinematic_1d.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ def init_attributes(self, *,
def sync(self):
super().sync()

# TODO: common with 2D
# TODO #418: common with 2D
@property
def dv(self):
return self.mesh.dv
2 changes: 1 addition & 1 deletion PySDM/environments/kinematic_2d.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ def init_attributes(self, *,
enable_temperatures=False,
rtol=default_rtol
):
# TODO #346 move to one method
# TODO #418 move to one method
super().sync()
self.notify()

Expand Down
2 changes: 1 addition & 1 deletion PySDM/physics/formulae.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ def rho_d(p, qv, theta_std):


class Hydrostatic:
# TODO: include assumptions in the name (p vs. pd)
# TODO #407: include assumptions in the name (p vs. pd)
@staticmethod
def drhod_dz(p, T, qv):
rho = MoistAir.rho_of_p_qv_T(p, qv, T)
Expand Down
2 changes: 1 addition & 1 deletion PySDM_examples/Arabas_and_Shima_2017_Fig_5/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ def dt_max(self):
t_total = 2 * Settings.z_half / self.w_avg
result = t_total / Settings.n_output
if result < 1 * si.centimetre / si.second:
result /= 100 # TODO #304
result /= 100 # TODO #411
return result

n_output = 500
Expand Down
2 changes: 1 addition & 1 deletion PySDM_examples/Arabas_et_al_2015_Figs_8_9/demo_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ def dt(self):

@property
def n_steps(self):
return int(self.ui_simulation_time.value / self.ui_dt.value) # TODO
return int(self.ui_simulation_time.value / self.ui_dt.value) # TODO #413

ui_condensation_rtol_x = IntSlider(
value=np.log10(Settings.condensation_rtol_thd),
Expand Down
2 changes: 1 addition & 1 deletion PySDM_examples/Arabas_et_al_2015_Figs_8_9/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ def __init__(self):

@property
def n_steps(self) -> int:
return int(self.simulation_time / self.dt) #TODO
return int(self.simulation_time / self.dt) # TODO #413

@property
def steps_per_output_interval(self) -> int:
Expand Down
6 changes: 3 additions & 3 deletions PySDM_examples/Bartman_et_al_2021/demo.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@
"settings.dt = 16 * si.second\n",
"settings.simulation_time = 1.75 * settings.spin_up_time\n",
"settings.output_interval = 1 * si.minute\n",
"settings.condensation_rtol_x = 1e-3 # TODO: needed to avoid timeout on Travis, revisit\n",
"settings.condensation_rtol_x = 1e-3 # TODO #419: needed to avoid timeout on Travis, revisit\n",
"settings.condensation_rtol_thd = 1e-3\n",
"\n",
"storage = Storage()\n",
Expand Down Expand Up @@ -175,7 +175,7 @@
"gs.tight_layout(fig)\n",
"ncdf = netcdf_file(temp_file.absolute_path, mode='r', mmap=False)\n",
"\n",
"# TODO: we should not use products here - all info should be obtained from netCDF\n",
"# TODO #419: we should not use products here - all info should be obtained from netCDF\n",
"plots = (\n",
" _ImagePlot(fig, axs[0], grid=ncdf.grid, size=ncdf.size, product=products[0], cmap='summer'),\n",
" _ImagePlot(fig, axs[1], grid=ncdf.grid, size=ncdf.size, product=products[1], cmap='PuBuGn'),\n",
Expand Down Expand Up @@ -264,4 +264,4 @@
},
"nbformat": 4,
"nbformat_minor": 4
}
}
4 changes: 2 additions & 2 deletions PySDM_examples/Shima_et_al_2009_Fig_2/demo.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@
"if GPU.ENABLE:\n",
" options.append(gpu)\n",
"boxes = widgets.HBox(options)\n",
"# TODO: freezer\n",
"# TODO #410: freezer\n",
"self = widgets.interactive_output(demo, \n",
" {'n_SD': n_SD, 'n_step': n_step, 'n_plot': n_plot, 'adaptive': adaptive, 'smooth': smooth, 'gpu': gpu})\n",
"\n",
Expand Down Expand Up @@ -245,4 +245,4 @@
},
"nbformat": 4,
"nbformat_minor": 4
}
}
2 changes: 1 addition & 1 deletion PySDM_examples/Shima_et_al_2009_Fig_2/example.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ def main(plot: bool, save: str):
plotter.smooth = True
for step, vals in states.items():
error = plotter.plot(vals, step * settings.dt)
#assert error < 200
#assert error < 200 # TODO #327
if save is not None:
n_sd = settings.n_sd
plotter.save(save + "/" +
Expand Down
4 changes: 2 additions & 2 deletions PySDM_examples/Shipway_and_Hill_2012/demo.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
"#### based on Fig. 1 from Shipway & Hill 2012 (Q. J. Royal Meteo. Soc. 138) \"_Diagnosis of systematic differences between multiple parametrizations of warm rain microphysics using a kinematic framework_\" \n",
"https://doi.org/10.1002/qj.1913\n",
"\n",
"**WARNING: TODO - particles are not advected by the flow yet (only sedimentation causes displacement)**"
"**WARNING: TODO #414 - particles are not advected by the flow yet (only sedimentation causes displacement)**"
]
},
{
Expand Down Expand Up @@ -301,4 +301,4 @@
},
"nbformat": 4,
"nbformat_minor": 1
}
}
2 changes: 1 addition & 1 deletion PySDM_examples/Shipway_and_Hill_2012/mpdata_1d.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ def __init__(self, nz, dt, advector_of_t, advectee_of_zZ_at_t0, g_factor_of_zZ,
def __call__(self):
self.t += .5 * self.dt
self.solver.advector.get_component(0)[:] = self.advector_of_t(self.t)
try: # TODO: move the try-except logic to within PyMPDATA
try: # TODO #417: move the try-except logic to within PyMPDATA
self.solver.advance(1)
except NumbaExperimentalFeatureWarning:
pass
Expand Down
8 changes: 4 additions & 4 deletions PySDM_examples/Shipway_and_Hill_2012/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@
class Settings:
def __init__(self, n_sd_per_gridbox, w_1=2*si.m/si.s, dt=1*si.s, dz=25*si.m, precip=True):
self.n_sd_per_gridbox = n_sd_per_gridbox
self.kappa = .9 # TODO: not in the paper
self.kappa = .9 # TODO #414: not in the paper
self.wet_radius_spectrum_per_mass_of_dry_air = Lognormal(
norm_factor=50/si.cm**3, # TODO: / self.rho,
norm_factor=50/si.cm**3, # TODO #414: / self.rho,
m_mode=.08/2 * si.um,
s_geom=1.4
)
Expand All @@ -29,10 +29,10 @@ def __init__(self, n_sd_per_gridbox, w_1=2*si.m/si.s, dt=1*si.s, dz=25*si.m, pre
self.w = lambda t: w_1 * np.sin(np.pi * t/t_1) if t < t_1 else 0

self._th = interp1d((0, 740, 3260), (297.9, 297.9, 312.66))
self.qv = interp1d((0, 740, 3260), (.015, .0138, .0024)) # TODO: is initial particle water included in initial qv? (q1 logic)
self.qv = interp1d((0, 740, 3260), (.015, .0138, .0024)) # TODO #414: is initial particle water included in initial qv? (q1 logic)
self.thd = lambda z: phys.th_dry(self._th(z), self.qv(z))

p0 = 975 * si.hPa # TODO: not in the paper?
p0 = 975 * si.hPa # TODO #414: not in the paper?
self.rhod0 = phys.ThStd.rho_d(p0, self.qv(0), self._th(0))

def drhod_dz(z, rhod):
Expand Down
2 changes: 1 addition & 1 deletion PySDM_examples/Shipway_and_Hill_2012/simulation.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ def __init__(self, settings, backend=CPU):
kernel=Geometric(collection_efficiency=1),
adaptive=settings.coalescence_adaptive
))
builder.add_dynamic(Displacement(enable_sedimentation=True, courant_field=(np.zeros(settings.nz+1),))) # TODO
builder.add_dynamic(Displacement(enable_sedimentation=True, courant_field=(np.zeros(settings.nz+1),))) # TODO #414
attributes = env.init_attributes(
spatial_discretisation=spatial_sampling.Pseudorandom(),
spectral_discretisation=spectral_sampling.ConstantMultiplicity(
Expand Down
4 changes: 2 additions & 2 deletions PySDM_examples/Yang_et_al_2018_Fig_2/demo.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@
"metadata": {},
"outputs": [],
"source": [
"def rmean(r, n, mask): # TODO: move to products\n",
"def rmean(r, n, mask): # TODO #412: move to products\n",
" nt = r.shape[1]\n",
" n_dot_r = n.magnitude.dot(np.where(mask, r.magnitude, 0))\n",
" n_tot = np.sum(np.where(mask, n.magnitude.reshape(-1,1).repeat(nt, axis=1), 0), axis=0)\n",
Expand Down Expand Up @@ -395,4 +395,4 @@
},
"nbformat": 4,
"nbformat_minor": 4
}
}
64 changes: 64 additions & 0 deletions PySDM_tests/devops_tests/test_todos_annotated.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
import pytest
import os
import re
import sys
import pathlib
from ghapi.all import GhApi, paged
from fastcore.net import ExceptionsHTTP


# https://stackoverflow.com/questions/7012921/recursive-grep-using-python
def findfiles(path, regex):
regObj = re.compile(regex)
res = []
for root, dirs, fnames in os.walk(path):
for fname in fnames:
if regObj.match(fname):
res.append(os.path.join(root, fname))
return res


def grep(filepath, regex):
regObj = re.compile(regex)
res = []
with open(filepath, encoding="utf8") as f:
for line in f:
if regObj.match(line):
res.append(line)
return res


@pytest.fixture(params=findfiles(pathlib.Path(__file__).parent.parent.parent.absolute(), r'.*\.(ipynb|py|txt|yml|m|jl|md)$'))
def file(request):
return request.param


@pytest.fixture(scope='session')
def gh_issues():
res = {}
try:
api = GhApi(owner='atmos-cloud-sim-uj', repo='PySDM')
pages = paged(api.issues.list_for_repo, owner='atmos-cloud-sim-uj', repo='PySDM', state='all', per_page=100)
for page in pages:
for item in page.items:
res[item.number] = item.state
except ExceptionsHTTP[403]:
pass
return res


@pytest.mark.skipif("'CI' in os.environ and ('GITHUB_ACTIONS' not in os.environ or sys.version_info.minor < 8)")
def test_todos_annotated(file, gh_issues):
if os.path.basename(file) == 'test_todos_annotated.py':
return
for line in grep(file, r'.*TODO.*'):
match = re.search(r'TODO #(\d+)', line)
if match is None:
raise Exception(f"TODO not annotated with issue id ({line})")
giving_up_with_hope_other_builds_did_it = len(gh_issues) == 0
if not giving_up_with_hope_other_builds_did_it:
number = int(match.group(1))
if number not in gh_issues.keys():
raise Exception(f"TODO annotated with non-existent id ({line})")
if gh_issues[number] != 'open':
raise Exception(f"TODO remains for a non-open issue ({line})")
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ def check(n_part, dv, n_sd, rho, state, step):
def test_coalescence(backend, croupier, adaptive):
if backend == ThrustRTC and croupier == 'local': # TODO #358
return
if backend == ThrustRTC and adaptive and croupier == 'global': # TODO #380
if backend == ThrustRTC and adaptive and croupier == 'global': # TODO #329
return
# Arrange
n_sd = 2 ** 14
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,4 @@ def test_qv():
@staticmethod
def test_rhod():
settings = Settings(n_sd_per_gridbox=1, w_1=1)
assert settings.rhod # TODO
assert settings.rhod # TODO #414
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@
import os


# TODO #304 run for different atol, rtol, dt_max
# TODO #411
if 'CI' in os.environ and not os.environ.get('FAST_TESTS') == 'true':
scheme = ('default', 'BDF')
scheme = ('default', 'BDF')
coord = ('volume logarithm', 'volume')
adaptive = (True, False)
enable_particle_temperatures = (False, True)
Expand Down
Loading

0 comments on commit 244a8de

Please sign in to comment.