Skip to content

reordering dimension #302

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.

Sign up for GitHub

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

Jump to bottom
Open
weiyuan-jiang wants to merge 8 commits into
base: develop
Choose a base branch
from
Open

reordering dimension #302

weiyuan-jiang wants to merge 8 commits into from

Conversation

@weiyuan-jiang
Copy link
Contributor

@weiyuan-jiang weiyuan-jiang commented Feb 14, 2025

This PR addressed issue #301

@weiyuan-jiang weiyuan-jiang added the 0 diff The changes in this pull request have verified to be zero-diff with the target branch. label Feb 14, 2025
@weiyuan-jiang weiyuan-jiang requested a review from a team as a code owner February 14, 2025 18:42
@amdasilva
Copy link
Contributor

amdasilva commented Mar 26, 2025

Have you tested it to very zero diff?

amdasilva
amdasilva suggested changes Mar 26, 2025
View reviewed changes
Copy link
Contributor

@amdasilva amdasilva left a comment

Choose a reason for hiding this comment

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

Let's talk, I don't think this is correct. The dimensioning of the arrays in the Fortran code should be changed to match the new order in the NetCDF file (of course, the data layout is what should should be the same, the order of dimensions is reversed because of the different ways Fortran and NetCDF/C index arrays.) It appears that you are reordering the array read from the file, and keeping the old Fortran ordering.

@weiyuan-jiang weiyuan-jiang mentioned this pull request Mar 28, 2025
Closed
@weiyuan-jiang
Copy link
Contributor Author

weiyuan-jiang commented Apr 1, 2025

This PR is zero diff with this branch feature/wjiang/dev_for_compare , which is slightly different from the develop branch. In develop branch, it uses some files from /discover/nobackup/pcolarco/fvInput/AeroCom/x/ which is not reordered

@weiyuan-jiang weiyuan-jiang added the Contingent - DNA Do Not Approve (DNA). These changes are contingent on other PRs label Apr 1, 2025
@github-actions
Copy link

github-actions bot commented Apr 1, 2025

Label error. Requires exactly 0 of: Contingent - DNA, Needs Lead Approval, Contingent -- Do Not Approve. Found: 0 diff, Contingent - DNA

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@amdasilva amdasilva amdasilva requested changes

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

0 diff The changes in this pull request have verified to be zero-diff with the target branch. Contingent - DNA Do Not Approve (DNA). These changes are contingent on other PRs

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@weiyuan-jiang @amdasilva