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

Fix single-point matrixCN fails #2840

Merged

Conversation

slevis-lmwg
Copy link
Contributor

Description of changes

Change SparseMatrixMultiplyMod.F90 line 1246:
SHR_ASSERT_FL((size(filter_actunit_C) > num_actunit_C), sourcefile, __LINE__)
to
SHR_ASSERT_FL((size(filter_actunit_C) >= num_actunit_C), sourcefile, __LINE__)
to help some failing matrixCN tests pass.

Specific notes

Contributors other than yourself, if any:
@ekluzek

CTSM Issues Fixed (include github issue #):
Fix #2780

Are answers expected to change (and if so in what way)?
No.

Any User Interface Changes (namelist or namelist defaults changes)?
No.

Does this create a need to change or add documentation? Did you do so?
No.

Testing performed, if any:
Failing tests now pass (see issue #2780).

@slevis-lmwg slevis-lmwg self-assigned this Oct 18, 2024
@slevis-lmwg slevis-lmwg added bug something is working incorrectly PR status: ready PR: this is ready to merge in, with all tests satisfactory and reviews complete bfb bit-for-bit size: small labels Oct 18, 2024
@slevis-lmwg
Copy link
Contributor Author

slevis-lmwg commented Oct 21, 2024

Rebase to cesm3_0_beta04_changes

Copy link
Collaborator

@ekluzek ekluzek left a comment

Choose a reason for hiding this comment

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

Thanks for this. Just needs to be done elsewhere as well≥

src/utils/SparseMatrixMultiplyMod.F90 Show resolved Hide resolved
@ekluzek ekluzek self-requested a review October 21, 2024 17:57
Copy link
Collaborator

@ekluzek ekluzek left a comment

Choose a reason for hiding this comment

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

We realized this should be done elsewhere, so I'll look at it again when this is done to approve again.

@slevis-lmwg slevis-lmwg changed the base branch from master to cesm3_0_beta04_changes October 22, 2024 23:18
@slevis-lmwg slevis-lmwg requested a review from ekluzek October 22, 2024 23:25
@slevis-lmwg
Copy link
Contributor Author

slevis-lmwg commented Oct 22, 2024

Testing on derecho and izumi in prep for merge:
OK ./run_sys_tests -s matrixcn -c ctsm5.3.009 --skip-generate

@slevis-lmwg
Copy link
Contributor Author

@ekluzek this is ready for me to merge with your approval.

@slevis-lmwg slevis-lmwg added the PR status: awaiting review Work on this PR is paused while waiting for review. label Oct 24, 2024
Copy link
Collaborator

@ekluzek ekluzek left a comment

Choose a reason for hiding this comment

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

Awesome this is great.

@slevis-lmwg slevis-lmwg dismissed ekluzek’s stale review November 8, 2024 18:54

Erik, you approved this a few minutes ago, but this review is still blocking the merge. I'm dismissing your review in case it unblocks the merge.

@slevis-lmwg
Copy link
Contributor Author

@ekluzek for now this is still blocked from merge...

slevis resolved conflicts:
cime_config/testdefs/ExpectedTestFails.xml
@slevis-lmwg
Copy link
Contributor Author

For the record:
I'm temporarily changing the beta04 branch settings to bypass the need for another review and approval.

@slevis-lmwg slevis-lmwg merged commit 3d811ca into ESCOMP:cesm3_0_beta04_changes Nov 8, 2024
2 checks passed
@slevis-lmwg slevis-lmwg deleted the fix_1x1_matrix_fails branch November 8, 2024 19:14
@slevis-lmwg
Copy link
Contributor Author

I have changed back the settings to require a review and approval.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bfb bit-for-bit bug something is working incorrectly PR status: awaiting review Work on this PR is paused while waiting for review. PR status: ready PR: this is ready to merge in, with all tests satisfactory and reviews complete size: small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New CN matrix fails with single point sites with the new ctsm5.3 datasets.
2 participants