Skip to content

Conversation

@AntSimi
Copy link
Owner

@AntSimi AntSimi commented Jun 18, 2021

No description provided.

@AntSimi AntSimi requested a review from ludwigVonKoopa June 18, 2021 07:32
@AntSimi AntSimi linked an issue Jun 18, 2021 that may be closed by this pull request
@AntSimi AntSimi marked this pull request as ready for review June 22, 2021 20:11
@AntSimi AntSimi requested a review from CoriPegliasco June 23, 2021 12:27
Copy link
Collaborator

@CoriPegliasco CoriPegliasco left a comment

Choose a reason for hiding this comment

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

English modifs :
In CHANGELOG.rst

  • Now time allows second precision (instead of daily precision) in storage on uint32 from 01/01/1950 to 01/01/2086
    New identifications are produced with this type, old files could still be loaded.
    If you use old identifications for tracking use the --unraw option to unpack old times and store data with the new format.
  • Now amplitude is stored with .1 mm of precision (instead of 1 mm), same advice as for time.

Changes in doc/run_tracking.rst
L8 : Before tracking, ...
L10 : Before tracking, display some identification files. You will learn a lot.
L27: Number of consecutive timesteps with missing detection allowed
L29 : Minimal number of timesteps to be considered as a long trajectory
L70 : same as L27
L72 : same as L 29

In share/tracking.yaml
L2 : Files produced ...
L4 : Path to save outputs
L7 : Minimal number of timesteps to be considered as a long trajectory
L9 : Number of consecutive timesteps with missing detection allowed

Questions :
In src/py_eddy_tracker/appli/eddies.py
L284 : why the exception is "several days steps"?

I think you know better than me what needed change to take into account the precision changes :)

@AntSimi AntSimi requested a review from CoriPegliasco June 25, 2021 19:44
@ludwigVonKoopa ludwigVonKoopa dismissed their stale review June 28, 2021 10:43

didn't take into account further commits

Copy link
Collaborator

@ludwigVonKoopa ludwigVonKoopa left a comment

Choose a reason for hiding this comment

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

some comments on the code,

but as it is an important change, and affect a lot of module / class / functions, we should implements some tests ?

@AntSimi
Copy link
Owner Author

AntSimi commented Jun 28, 2021

but as it is an important change, and affect a lot of module / class / functions, we should implements some tests ?

I run tests from detection to final tracking with grid file with 3h step. For now i didn't imagine other tests.

@AntSimi AntSimi requested a review from ludwigVonKoopa June 28, 2021 20:41
@ludwigVonKoopa ludwigVonKoopa merged commit ab3020f into master Jun 29, 2021
@AntSimi AntSimi deleted the time_float branch September 2, 2021 10:57
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.

Eddy tracking on 3-hourly files

4 participants