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 an error about bs550aer #468

Merged
merged 6 commits into from
Apr 23, 2019
Merged

Fix an error about bs550aer #468

merged 6 commits into from
Apr 23, 2019

Conversation

kjoti
Copy link
Contributor

@kjoti kjoti commented Apr 4, 2019

This error occurs if a variable has two or more singleton
dimensions and these axis ids (axes_ids) are omitted
when invoking cmor_variable().

This error occurs if a variable has two or more singleton
dimensions and these axis ids (axes_ids) are omitted
when invoking cmor_variable().
@doutriaux1
Copy link
Collaborator

@kjoti please add a test case. @mauzey1 please review as well.

mauzey1
mauzey1 previously approved these changes Apr 11, 2019
@mauzey1 mauzey1 self-requested a review April 11, 2019 22:47
@mauzey1 mauzey1 dismissed their stale review April 11, 2019 23:17

The test doesn't fail when I try removing the + lndims. I can see that the changes do automatically include both singletons in the NetCDF files, but the test doesn't detect that.

@kjoti
Copy link
Contributor Author

kjoti commented Apr 12, 2019

@mauzey1
It's mysterious.

There are three tests in Test/test_singletons.c. Even when removing + lndims, the 2nd and 3rd tests will succeed, but the 1st should fail.

Please check output files for each test if your test execution terminates normally. In my execution, a segmentation fault has occurred in the 1st test if + lndims is removed.

@mauzey1
Copy link
Collaborator

mauzey1 commented Apr 19, 2019

@kjoti I have added some suggested changes to the test. The changes will allow the test to see if the singleton dimensions were added to the variable. You can commit all three suggestions as a batch.

Also, please update your branch with the latest from master.

mauzey1 and others added 3 commits April 22, 2019 14:08
@kjoti
Copy link
Contributor Author

kjoti commented Apr 22, 2019

@mauzey1
Thank you for your suggestion.

@mauzey1 mauzey1 merged commit 53ce99f into PCMDI:master Apr 23, 2019
@mauzey1
Copy link
Collaborator

mauzey1 commented Apr 23, 2019

Fixed #464

@wachsylon
Copy link
Collaborator

Hi @mauzey1,
for me the
Test/test_singletons.c
is the only test that fails for the 3.6.0 release when I use the 6.9.32 Tables. It says:

! Error: You are defining variable 'bs550aer' (table 6hrLev) with 3 dimensions, when it should have 5

Is there something that I need to change?
Best regards,
Fabi

@mauzey1
Copy link
Collaborator

mauzey1 commented Sep 4, 2020

@wachsylon In the 6.9.32 tables, we removed the scatter180 dimension from the variable bs550aer. We then changed test_singleton.c to reflect this change.

I have tested test_singleton.c using the latest build of CMOR 3.6.0 and the 6.9.32 tables and did not get the error. Check if your test file is the latest from the repo, and make sure that it is using the directory with the 6.9.32 table.

@wachsylon
Copy link
Collaborator

Thanks, it works with the recent test.

Maybe it is possible to design these tests independently from the tables. The failing test suggests that the cmor version is incompatible with the table version which is probably not true.

Another idea would be to tag a cmor 3.6.1. But I know this is a lot of effort for one test...

@kjoti
Copy link
Contributor Author

kjoti commented Sep 8, 2020

This test was for multiple singleton axes. bs550aer was the only case of having multiple singleton axes (lambda550nm and scatter180).

Since scatter180 has been removed from bs550aer, this test becomes useless and can be discarded.

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.

5 participants