Complex eof#9
Merged
Merged
Conversation
…izes the data as time vs space and subtract the mean field in each coordinate. It returns all the variables related to CEOF in a DataArray.
I added the description for the ceof and reconstruct_ceof modules.
I added the ceof, reconstruct_ceof modules in the init file
MariaMigl
reviewed
Jun 26, 2021
| # PERFORM COMPLEX EOF | ||
| #========================================= | ||
| from scipy.signal import hilbert | ||
| import scipy.linalg as la |
Collaborator
There was a problem hiding this comment.
Imports should be placed in the top of each file regardless of the fuction in which they are used
dantecn
reviewed
Jul 12, 2021
| Translated to Python by Felipe Vilela da Silva @ IMAS, UTas, AU on 15/Mar/2021 | ||
| ============================================================================== | ||
| Inputs: | ||
| -> lon: longitudes (array) |
Member
There was a problem hiding this comment.
Can you make the inputs following the standards of the other .py? (Maybe it changed after you started this) It's just capital input/output and the variables below the "U" (3 spaces), with the "=" signed aligned for the larger name: Example INPUT: Small = A variable with a small name BigNamedOne = A variable with a big name
dantecn
reviewed
Jul 12, 2021
| -> pcs: First nkp Complex Principal Components or amplitudes [time, nkp] | ||
| -> t_amp: Temporal amplitude [time, nkp] | ||
| -> t_phase: Temporal phase [time, nkp] | ||
| ============================================================================== |
Member
There was a problem hiding this comment.
Please, remove all authorship comments (except for previous credits or paper citations). All contributors from OceanLab Team will be included in a single "contributors" file
Following Ryukamusa request about "Imports should be placed in the top of each file"
Following dantecn request
Now, org_data_ceof works faster and returns a DataArray
Followed @dantecn suggestion.
I just realized a typo.
dantecn
reviewed
Jul 13, 2021
| New modifications: | ||
| (i) The function organizes the data as time vs space and subtract the mean field in each coordinate | ||
| (ii) It returns all the variables related to CEOF in a DataArray | ||
| ''' |
Member
There was a problem hiding this comment.
I'm not sure this version control is useful here.
- github has version control
- when we merge these commits, it will be the "actual first version" =P
| coords={'lat':(dims[1], lat), 'lon':(dims[2], lon)}) | ||
| data_ceof = datxarray.stack(lat_lon=("lat", "lon")).data_latlon | ||
| return data_ceof | ||
|
|
Member
There was a problem hiding this comment.
@Ryukamusa @iuryt any idea on how to define these functions that are used only within other functions and used only internally?
Maybe define with the underline "_" at the beginning, as we did for _parallel_computing in the utils module?
def _org_data_ceof()
def _amplitude_phase()
Member
There was a problem hiding this comment.
I have to learn where to put my comments o.0
I just followed dantecn suggestion
"_" at the beginning of auxiliary functions.
I added the parallel option to build a standard client within the ceof function. In case the user doesn't want a standard client, he/she can turn the option to False and build the client with other settings.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The ceof and reconstruct_ceof functions were tested in two different datasets (array and DataArray) in two different regions of the ocean. The ceof() returns the eigenvalues, eigenvectors (real and imaginary), amplitudes (real and imaginary), besides the spatial and temporal amplitudes and phases in a DataArray. The reconstruct_ceof() gives the reconstrucion of CEOF modes individually. I am happy to hear and follow any review.