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

Handle writing of empty arrays to preCICE #69

Merged
merged 10 commits into from
Nov 18, 2020

Conversation

IshaanDesai
Copy link
Member

In a parallel coupling case, ranks which have no part of the coupling interface will call the write data functionality with empty data structures. The bindings need to handle this as the main preCICE API allows this.

@IshaanDesai IshaanDesai self-assigned this Nov 16, 2020
@IshaanDesai IshaanDesai added the bug Something isn't working label Nov 16, 2020
@BenjaminRodenberg BenjaminRodenberg marked this pull request as draft November 17, 2020 09:19
@BenjaminRodenberg
Copy link
Member

I converted this PR to a draft, since there are still some errors in the source code that the tests run into (see Travis). Do you already want me to do a review now or do you first fix the errors?

Copy link
Member

@BenjaminRodenberg BenjaminRodenberg left a comment

Choose a reason for hiding this comment

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

There are some formatting changes in setup.py. If they are necessary due to code style, please directly commit them to develop. Don't add them to this PR.

@IshaanDesai
Copy link
Member Author

There are some formatting changes in setup.py. If they are necessary due to code style, please directly commit them to develop. Don't add them to this PR.

Formatting changes to setup.py were introduced unintentionally during a merge conflict resolution, sorry about that. The reformatting is now done in develop and then develop is merged into this branch.

@IshaanDesai
Copy link
Member Author

I converted this PR to a draft, since there are still some errors in the source code that the tests run into (see Travis). Do you already want me to do a review now or do you first fix the errors?

The error in tests was just due to a small spelling mistake. It is now rectified and PR is ready for review 👍

@IshaanDesai IshaanDesai marked this pull request as ready for review November 17, 2020 11:26
Copy link
Member

@BenjaminRodenberg BenjaminRodenberg left a comment

Choose a reason for hiding this comment

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

Looks good. Minor point regarding the non-block operations. I also added some tests in 86fc5d4 that worked nicely.

I also checked that the new tests actually fail for the current state of develop and got the following error:

======================================================================
ERROR: test_read_write_block_scalar_data_empty (test_bindings_module.TestBindings)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "test/test_bindings_module.pyx", line 201, in test_bindings_module.TestBindings.test_read_write_block_scalar_data_empty
    solver_interface.write_block_scalar_data(1, [], write_data)
  File "precice.pyx", line 952, in precice.Interface.write_block_scalar_data
    size = vertex_ids.size
AttributeError: 'list' object has no attribute 'size'

======================================================================
ERROR: test_read_write_block_vector_data_empty (test_bindings_module.TestBindings)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "test/test_bindings_module.pyx", line 237, in test_bindings_module.TestBindings.test_read_write_block_vector_data_empty
    solver_interface.write_block_vector_data(1, [], write_data)
  File "precice.pyx", line 871, in precice.Interface.write_block_vector_data
    size, dimensions = values.shape
ValueError: need more than 1 value to unpack

----------------------------------------------------------------------

precice.pyx Outdated Show resolved Hide resolved
@IshaanDesai IshaanDesai merged commit 04a6639 into develop Nov 18, 2020
@IshaanDesai IshaanDesai deleted the handleZeroValuedWriteData branch November 18, 2020 16:21
BenjaminRodenberg added a commit that referenced this pull request Dec 28, 2020
* Correcting spelling mistake

Co-authored-by: Benjamin Rüth <[email protected]>

Co-authored-by: BenjaminRueth <[email protected]>
@BenjaminRodenberg BenjaminRodenberg mentioned this pull request Dec 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants