stfc / HartreeParticleDSL

MIT License
1 stars 1 forks source link

New branch for source and sink boundary implementation #68

Closed LonelyCat124 closed 1 year ago

LonelyCat124 commented 1 year ago

This PR contains the draft implementation for source and sink boundaries, as needed for the miniapp proposed by UKAEA.

So far this contains just a kernel definition, and the PIR nodes to represent them in the tree. Next is to add the tree to PIR converted, and then add support for them in Cabana_PIR backend.

Miniapps showing example implementions with Cabana are done, so hopefully it shouldn't be too complex to do this.

LonelyCat124 commented 1 year ago

I think the required implementation is now in the code.

Remaining things I need is:

LonelyCat124 commented 1 year ago

For random number then we need something in get_extra_symbols to add a new structure if random_number is found, and to return generator, which needs to be of type auto and set to an initial value somewhere/how, the setting up and removal of generator is more difficult.

LonelyCat124 commented 1 year ago

Created a new AutoSymbol for creating auto style C++ types. This I can use for the generator maybe.

LonelyCat124 commented 1 year ago

Support for random number is "done". Testing etc. still required.

LonelyCat124 commented 1 year ago

add_include already exist and works for 2) anyway.

LonelyCat124 commented 1 year ago

Ok, a basic implementation now works.

LonelyCat124 commented 1 year ago

I think ideally we don't want to do MPI communication for deletion flags but thats is not yet implemented.

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (ea7012d) 100.00% compared to head (870c520) 100.00%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #68 +/- ## ========================================== Coverage 100.00% 100.00% ========================================== Files 68 72 +4 Lines 6834 7606 +772 ========================================== + Hits 6834 7606 +772 ``` | [Impacted Files](https://codecov.io/gh/stfc/HartreeParticleDSL/pull/68?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=stfc) | Coverage Δ | | |---|---|---| | [src/HartreeParticleDSL/HartreeParticleDSL.py](https://codecov.io/gh/stfc/HartreeParticleDSL/pull/68?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=stfc#diff-c3JjL0hhcnRyZWVQYXJ0aWNsZURTTC9IYXJ0cmVlUGFydGljbGVEU0wucHk=) | `100.00% <100.00%> (ø)` | | | [...HartreeParticleDSL/IO\_modules/PHDF5\_IO/PHDF5\_IO.py](https://codecov.io/gh/stfc/HartreeParticleDSL/pull/68?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=stfc#diff-c3JjL0hhcnRyZWVQYXJ0aWNsZURTTC9JT19tb2R1bGVzL1BIREY1X0lPL1BIREY1X0lPLnB5) | `100.00% <100.00%> (ø)` | | | [...ParticleDSL/IO\_modules/base\_IO\_module/IO\_module.py](https://codecov.io/gh/stfc/HartreeParticleDSL/pull/68?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=stfc#diff-c3JjL0hhcnRyZWVQYXJ0aWNsZURTTC9JT19tb2R1bGVzL2Jhc2VfSU9fbW9kdWxlL0lPX21vZHVsZS5weQ==) | `100.00% <100.00%> (ø)` | | | [...rtreeParticleDSL/Particle\_IR/datatypes/datatype.py](https://codecov.io/gh/stfc/HartreeParticleDSL/pull/68?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=stfc#diff-c3JjL0hhcnRyZWVQYXJ0aWNsZURTTC9QYXJ0aWNsZV9JUi9kYXRhdHlwZXMvZGF0YXR5cGUucHk=) | `100.00% <100.00%> (ø)` | | | [...reeParticleDSL/Particle\_IR/nodes/auto\_reference.py](https://codecov.io/gh/stfc/HartreeParticleDSL/pull/68?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=stfc#diff-c3JjL0hhcnRyZWVQYXJ0aWNsZURTTC9QYXJ0aWNsZV9JUi9ub2Rlcy9hdXRvX3JlZmVyZW5jZS5weQ==) | `100.00% <100.00%> (ø)` | | | [...rc/HartreeParticleDSL/Particle\_IR/nodes/kernels.py](https://codecov.io/gh/stfc/HartreeParticleDSL/pull/68?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=stfc#diff-c3JjL0hhcnRyZWVQYXJ0aWNsZURTTC9QYXJ0aWNsZV9JUi9ub2Rlcy9rZXJuZWxzLnB5) | `100.00% <100.00%> (ø)` | | | [...leDSL/Particle\_IR/nodes/particle\_core\_reference.py](https://codecov.io/gh/stfc/HartreeParticleDSL/pull/68?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=stfc#diff-c3JjL0hhcnRyZWVQYXJ0aWNsZURTTC9QYXJ0aWNsZV9JUi9ub2Rlcy9wYXJ0aWNsZV9jb3JlX3JlZmVyZW5jZS5weQ==) | `100.00% <100.00%> (ø)` | | | [...rtreeParticleDSL/Particle\_IR/symbols/autosymbol.py](https://codecov.io/gh/stfc/HartreeParticleDSL/pull/68?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=stfc#diff-c3JjL0hhcnRyZWVQYXJ0aWNsZURTTC9QYXJ0aWNsZV9JUi9zeW1ib2xzL2F1dG9zeW1ib2wucHk=) | `100.00% <100.00%> (ø)` | | | [...backends/AST\_to\_Particle\_IR/ast\_to\_pir\_visitors.py](https://codecov.io/gh/stfc/HartreeParticleDSL/pull/68?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=stfc#diff-c3JjL0hhcnRyZWVQYXJ0aWNsZURTTC9iYWNrZW5kcy9BU1RfdG9fUGFydGljbGVfSVIvYXN0X3RvX3Bpcl92aXNpdG9ycy5weQ==) | `100.00% <100.00%> (ø)` | | | [...ticleDSL/backends/Cabana\_PIR\_backend/cabana\_pir.py](https://codecov.io/gh/stfc/HartreeParticleDSL/pull/68?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=stfc#diff-c3JjL0hhcnRyZWVQYXJ0aWNsZURTTC9iYWNrZW5kcy9DYWJhbmFfUElSX2JhY2tlbmQvY2FiYW5hX3Bpci5weQ==) | `100.00% <100.00%> (ø)` | | | ... and [6 more](https://codecov.io/gh/stfc/HartreeParticleDSL/pull/68?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=stfc) | | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=stfc). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=stfc)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

LonelyCat124 commented 1 year ago

Source boundaries are now just done by rank 0 for now if MPI is enabled.

LonelyCat124 commented 1 year ago

I created a very naive test for this and current issues:

  1. core_part_slice is being generated instead of core_part_velocity_slice I think.

The rest all seem related to that issue.

LonelyCat124 commented 1 year ago

I'm going to add another "particle_core_reference" node to Particle_IR to handle elements which are part of the core particle structure, but are not the position. The way these are described in the code differs from how general particle accesses work, so they're not currently handled correct by codegen.

LonelyCat124 commented 1 year ago

Ideally need a way to write to a variables/config member from a kernel. This is a bit tricky, I think it again needs a special node and a bit of analysis. It might be we need to create a manual kernel for now for the UKAEA use case.

LonelyCat124 commented 1 year ago

Alternatively an "add writeable array" is doable, and write to it with atomic add maybe works.

LonelyCat124 commented 1 year ago

Added writable arrays options, need to add atomic add functionality perhaps or some other way to handle it.

LonelyCat124 commented 1 year ago

Remaining problem with that code for compilation (pre-worrying about atomic or reduction accesses, which may have to be manual for now, but we'll see) is that the array is now printed in the symbol table for some reason, need to check why and remove it, and also array accesses to these arrays should be with () operator and not [] operator.

LonelyCat124 commented 1 year ago

Ok, it now uses the () operator for a view instead. Some of this code is not super nice but it works for now.

LonelyCat124 commented 1 year ago

The one remaining issue I see is that we need to specify where the lambda's that update the extra arrays are executed (to execute on the host and not the device). TBC how to do this.

LonelyCat124 commented 1 year ago

Probably need a global particle count accessible through the config?

LonelyCat124 commented 1 year ago

Setting of nparts is wrong.

LonelyCat124 commented 1 year ago

Can't reuse the generator so we need to do something better than that in codegen when using random numbers

LonelyCat124 commented 1 year ago

Missing a fence after sink boundary call, and also after source boundary call. Sink boundary not flagging for deletion seemingly.

LonelyCat124 commented 1 year ago

Small bug in the code that generated the deletion section, should check for == 0 instead of <= 0 I think. Also probably inserting 100k is enough to test for now as it reaches a steady state at around 1M parts - maybe 200k is better but not 1M.

LonelyCat124 commented 1 year ago

Still not working with GPU though.

Velocities are still 0...

Not to do with randomness, even if i set the values to a constant its not working correctly.

Kernels just don't work for GPU this time?!

LonelyCat124 commented 1 year ago

Shit the GPU kernels just don't work :(

Edit: The old manual implemetnation kernels DO do some updates so I'm a bit confused here, need to check whats going on.

LonelyCat124 commented 1 year ago

Super stupid, super easy fix. Don't forgot the KOKKOS_INLINE_FUNCTION before operators...else it breaks with CUDA :)

LonelyCat124 commented 1 year ago

Still an issue in the magnetic field not loading for the PIC case. Not deep copying the data into the fields...oops.

Thats now fixed, but no output values still? Other than Bz.

LonelyCat124 commented 1 year ago

Ok, this is pretty much ready to be merged in its current state i think. Tested fully, and runs on machines - performance of sink is bad but thats hard to resolve atm.