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

allow constructor for density free formular functor #3024

Conversation

psychocoderHPC
Copy link
Member

@psychocoderHPC psychocoderHPC commented Aug 16, 2019

FreeFormulaImpl allows the user functor to define a constructor to move additional information into the profile.
This allow e.g. filtering by cell position relative to the global volume

example

//! example implementation is not supporting moving window
struct RelativeRangeProfile
{
    static constexpr float_X start_ratio = 0.1_X;
    static constexpr float_X end_ratio = 0.3_X;
    static constexpr uint32_t usedDim = 1u;

    float_64 first_cell = 0.0_X;
    float_64 last_cell = 0.0_X;
    RelativeRangeProfile()
    {
        const SubGrid< simDim >& subGrid = Environment< simDim >::get().SubGrid();

        auto globalSize = subGrid.getGlobalDomain().size;
        auto deviceOffset = subGrid.getLocalDomain().offset;
        first_cell = ( static_cast< float_64 >( globalSize[usedDim] ) * start_ratio ) -
            static_cast< float_64 >( deviceOffset[ usedDim ] );
        last_cell = ( static_cast< float_64 >( globalSize[ usedDim ] ) * end_ratio ) -
            static_cast< float_64 >( deviceOffset[ usedDim ] );
    }

    //! homogenous density between [start_ratio;end_ratio) of the global domain
    HDINLINE float_X
    operator()(
        floatD_64 const & position_SI,
        float3_64 const & cellSize_SI
    )
    {
        return position_SI[usedDim] >= first_cell && position_SI[usedDim] < last_cell ? 1.0_X : 0.0_X;
    }
};

// definition of free formula profile
using HomogenousRange = FreeFormulaImpl< RelativeRangeProfile >;

`FreeFormulaImpl` allows the user functor to define a constructor to move additional information into the profile.
This allow e.g. filtering by cell position relative to the global volume
@psychocoderHPC psychocoderHPC added component: core in PIConGPU (core application) refactoring code change to improve performance or to unify a concept but does not change public API labels Aug 16, 2019
@PrometheusPi PrometheusPi self-assigned this Aug 16, 2019
@PrometheusPi
Copy link
Member

Great work - I will test it right away.

@sbastrakov
Copy link
Member

Looks good to me, not sure if @PrometheusPi has tried this out.

@PrometheusPi
Copy link
Member

@psychocoderHPC and @sbastrakov sorry I was busy with creating videos and did not test it yet - will do this tomorrow

@PrometheusPi
Copy link
Member

I am testing it right now.

@ax3l
Copy link
Member

ax3l commented Aug 30, 2019

@PrometheusPi how did the test go? :)

@PrometheusPi
Copy link
Member

Just an inbetween remark:
Where do these strange velocity fluctuations on the shear surface come from?
grafik

@PrometheusPi
Copy link
Member

PrometheusPi commented Sep 4, 2019

Just to illustrate this a bit further
grafik

Blue is the velocity in one x-direction - red in the other. This strange overlap at the shear surface should not look like this).

This has so far nothing to do with this pull request, but is the current status of the dev to check this pull request.

@PrometheusPi
Copy link
Member

PrometheusPi commented Sep 4, 2019

The issue seen in the isaac visualization was caused by an error on my input set and the strange behavior of the automatic range setting (see ComputationalRadiationPhysics/isaac#99).

@PrometheusPi
Copy link
Member

Setting density only in (..., [0.25, 0.75], ...) works fine:
grafik

PrometheusPi
PrometheusPi previously approved these changes Sep 4, 2019
Copy link
Member

@PrometheusPi PrometheusPi 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 to me.
Is the only algorithmic difference, that the constructor is now called by the template constructor?

@PrometheusPi
Copy link
Member

Works like a charm:
grafik

grafik

@sbastrakov
Copy link
Member

sbastrakov commented Sep 4, 2019

@PrometheusPi on your constructor question - yes, the idea is that with this PR one can pass arbitrary parameters to a constructor of the user-provided functor through the constructor of FreeFormulaImpl class.

Edit: upon a further look, it is not actually arbitrary but the choice is only between the default constructor and the one taking a uint32_t currentStep. However, it looks like support for arbitrary parameters can be added rather easily when needed.

@PrometheusPi
Copy link
Member

@sbastrakov Thank you for the explanation.

@PrometheusPi
Copy link
Member

@ComputationalRadiationPhysics/picongpu-maintainers after this pull request has been added - should the KHI example be refactored to use 4 instead of 2 species?

@PrometheusPi
Copy link
Member

@sbastrakov Since @psychocoderHPC is on vacation, I quickly fixed the typo.

@PrometheusPi
Copy link
Member

discussed offline with @sbastrakov:
The documentation of these "template concepts" needs to be done in a wider and more general scope. Thus we will need to postpone this to a future pull request.

@PrometheusPi PrometheusPi merged commit c520e83 into ComputationalRadiationPhysics:dev Sep 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: core in PIConGPU (core application) refactoring code change to improve performance or to unify a concept but does not change public API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants