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

refactor ListEntry used in collision #4811

Conversation

psychocoderHPC
Copy link
Member

@psychocoderHPC psychocoderHPC commented Feb 8, 2024

Refactor changes introduced in #4745 to avoid void using void pointer as a place holder for the later used frame list.

@psychocoderHPC psychocoderHPC added bug a bug in the project's code component: core in PIConGPU (core application) labels Feb 8, 2024
@psychocoderHPC psychocoderHPC added this to the 0.8.0 / Next stable milestone Feb 8, 2024
@psychocoderHPC psychocoderHPC marked this pull request as draft February 8, 2024 10:53
@pordyna
Copy link
Member

pordyna commented Feb 20, 2024

Still fails with:

Unhandled exception of type 'St13runtime_error' with message '/global/u2/p/pordyna/picongpu-setups/picongpu/thirdParty/cupla/alpaka/include/alpaka/event/EventUniformCudaHipRt.hpp(143) 'ret = TApi::eventQuery(event.getNativeHandle())' returned error  : 'cudaErrorIllegalAddress': 'an illegal memory access was encountered'!', terminating
terminate called after throwing an instance of 'std::runtime_error'
  what():  /global/u2/p/pordyna/picongpu-setups/picongpu/thirdParty/cupla/alpaka/include/alpaka/event/EventUniformCudaHipRt.hpp(190) 'TApi::eventSynchronize(event.getNativeHandle())' returned error  : 'cudaErrorIllegalAddress': 'an illegal memory access was encountered'!

Fix changes introduced in ComputationalRadiationPhysics#4745 to avoid invalid memory access.
Avoid using void pointer as place holder for the later used frame list.
@psychocoderHPC
Copy link
Member Author

We will merge this PR before going back to the slow particle access in collision #4810.

We need to test in a physical setup if #4828 + #4825 is solving the Perlmutter issues @pordyna is observing.

@psychocoderHPC psychocoderHPC marked this pull request as ready for review February 23, 2024 16:30
@psychocoderHPC psychocoderHPC changed the title fix collision particle invalid memory (Version 2) refactor ListEntry used in collision Feb 23, 2024
DINLINE auto operator[](uint32_t idx) const
{
const uint32_t inSuperCellIdx = m_parIdxList[idx];
return m_framePtrList[inSuperCellIdx / frameSize][inSuperCellIdx % frameSize];
Copy link
Member

Choose a reason for hiding this comment

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

Is it guaranteed that the division inSuperCellIdx / frameSize does not yield a floating point number? (A comment may state that)

Copy link
Member

Choose a reason for hiding this comment

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

With both integers? I don't think so

Copy link
Member Author

@psychocoderHPC psychocoderHPC Feb 26, 2024

Choose a reason for hiding this comment

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

Both types a integral types, they can not end in floating point numbers, that are defined by the language.
A comment is not useful because it will increase the code size without extra new knowledge.

Copy link
Contributor

@ikbuibui ikbuibui left a comment

Choose a reason for hiding this comment

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

Looks okay

@ikbuibui ikbuibui merged commit a48270a into ComputationalRadiationPhysics:dev Feb 27, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug a bug in the project's code component: core in PIConGPU (core application)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants