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 gameOfLife #2700

Merged

Conversation

psychocoderHPC
Copy link
Member

@psychocoderHPC psychocoderHPC commented Sep 5, 2018

The game of life example has a runtime issue that the mapping description verification
to check the number of supercells is always twrowing an error. The reason it that the
not existing default constructor is MappingDescription is used.
Please do not ask me why the code before is compiling. The class MappingDescription has no default constructor but is a not initialized member of the class Evolution<> which means by definition the default constructor should be called. Due to the fact that there is a user defined constructor there should be no default constructor available.

Error message:

./gameOfLife -d  1 1  -g 128 128 -p 1 1 -r 23/3 -s 100
newborn if=3 stay alive if=23 mask=4108
terminate called after throwing an instance of 'std::runtime_error'
  what():  expression (( guardingSuperCells[ d ] == 0 && gridSuperCells[ d ] >= 1) || gridSuperCells[ d ] >= 2 * guardingSuperCells[ d ] + 1) failed in file (/include/pmacc/mappings/kernel/MappingDescription.hpp:82) : 
Aborted
  • create the object for mapping description inside the init method of Evolution
  • change interface of Evolution<...>::init()
  • remove the default constructor for MappingDescription (it make no sense to allow a mapping with zero cells)

@psychocoderHPC psychocoderHPC added bug a bug in the project's code affects latest release a bug that affects the latest stable release labels Sep 5, 2018
@sbastrakov
Copy link
Member

sbastrakov commented Sep 6, 2018

@psychocoderHPC the constructor of MappingDescription has default values for both parameters, so it is also a default constructor and therefore previously the code has compiled successfully (but maybe it would be better if that did not compile, see my next comment).

sbastrakov
sbastrakov previously approved these changes Sep 6, 2018
Copy link
Member

@sbastrakov sbastrakov left a comment

Choose a reason for hiding this comment

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

I have nothing against these changes. However, doesn't the fact these are required mean that the default constructor of MappingDescription (default parameter values) is actually harmful as it initializes in an invalid state?

@psychocoderHPC
Copy link
Member Author

@sbastrakov Thanks I missed the fact that there is a default for all parameter. I fully agree that it should not MappingDescription should not be default constructable, this bug shows why.

I will fix it by changing the interface of MappingDescription

@ax3l ax3l added component: examples PIConGPU or PMacc examples component: PMacc in PMacc labels Sep 6, 2018
@ax3l ax3l self-assigned this Sep 6, 2018
@ax3l ax3l changed the title fix gameOfLife [WIP] fix gameOfLife Sep 6, 2018
The game of life example has a runtime issue that the mapping description verification
to check the number of supercells is always twrowing an error. The reason it that the
not existing default constructor is `MappingDescription` is used.

- create the object for mapping description inside the init method of `Evolution`
- `MappingDescription` remove the default for the parameter `localGridCells`
@psychocoderHPC psychocoderHPC changed the title [WIP] fix gameOfLife fix gameOfLife Sep 6, 2018
@psychocoderHPC
Copy link
Member Author

I removed the default constructor for MappingDescription

@ax3l ax3l mentioned this pull request Sep 6, 2018
8 tasks
@ax3l ax3l merged commit 2f265eb into ComputationalRadiationPhysics:dev Sep 6, 2018
@psychocoderHPC psychocoderHPC deleted the fix-gameOfLife branch September 6, 2018 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects latest release a bug that affects the latest stable release bug a bug in the project's code component: examples PIConGPU or PMacc examples component: PMacc in PMacc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants