Codecov Report

Base: 61.35% // Head: 61.56% // Increases project coverage by +0.21% :tada:

Coverage data is based on head (cfeac19) compared to base (921a975). Patch coverage: 29.41% of modified lines in pull request are covered.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #30 +/- ## ========================================== + Coverage 61.35% 61.56% +0.21% ========================================== Files 9 9 Lines 533 536 +3 ========================================== + Hits 327 330 +3 Misses 206 206 ``` | [Impacted Files](https://codecov.io/gh/snek5000/snek5000-cbox/pull/30?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=snek5000) | Coverage Δ | | |---|---|---| | [tests/test\_slow\_RB.py](https://codecov.io/gh/snek5000/snek5000-cbox/pull/30/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=snek5000#diff-dGVzdHMvdGVzdF9zbG93X1JCLnB5) | `14.15% <0.00%> (ø)` | | | [tests/test\_slow\_sidewall.py](https://codecov.io/gh/snek5000/snek5000-cbox/pull/30/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=snek5000#diff-dGVzdHMvdGVzdF9zbG93X3NpZGV3YWxsLnB5) | `12.80% <0.00%> (ø)` | | | [src/snek5000\_cbox/solver.py](https://codecov.io/gh/snek5000/snek5000-cbox/pull/30/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=snek5000#diff-c3JjL3NuZWs1MDAwX2Nib3gvc29sdmVyLnB5) | `100.00% <100.00%> (ø)` | | | [tests/conftest.py](https://codecov.io/gh/snek5000/snek5000-cbox/pull/30/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=snek5000#diff-dGVzdHMvY29uZnRlc3QucHk=) | `90.00% <100.00%> (ø)` | | | [tests/test\_init.py](https://codecov.io/gh/snek5000/snek5000-cbox/pull/30/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=snek5000#diff-dGVzdHMvdGVzdF9pbml0LnB5) | `100.00% <100.00%> (ø)` | | 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=snek5000). 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=snek5000)

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

akhoubani commented 1 year ago

@paugier It is ready to be merged.

paugier commented 1 year ago

I really think we should avoid params.oper.aspect_ratio. This parameter can easily be computed from other parameters since it is just params.oper.Ly / params.oper.Lx.

akhoubani commented 1 year ago

I really think we should avoid params.oper.aspect_ratio. This parameter can easily be computed from other parameters since it is just params.oper.Ly / params.oper.Lx.

I know params.oper.aspect_ratio is params.oper.Ly / params.oper.Lx. Do you know how I can communicate params.oper.Ly and params.oper.Lx to userbc and useric subroutines?

paugier commented 1 year ago

I don't know exactly how to obtain Lx in .usr functions. However, I know that it has to be very simple.

Lx is given to Nek5000 through the .box file, which is then used by the genbox utility. I don't know if it corresponds then to a variable in Nek5000.

if yes, this variable should be used.
if no, you have to create a Fortran global variable to store these data at initialization of the Fortran program and then use them when needed. I don't know from where they should be read in Fortran. @ashwinvis what do you think?

ashwinvis commented 1 year ago

I agree with @akhoubani that calculating Lx, Ly before the first time step is most likely not possible. Even in the phill example the lengths are hard coded in usrdat2.

I had similar problem in obtaining global number of elements and I ended up altering the SIZE.j2 template

      ! USER SPECIFIED: u_ prefix added to avoid name clash
      integer u_nelx, u_nely, u_nelz
      {{ parameter("u_nelx", params.oper.nx) }}  ! number of elements in x direction
      {{ parameter("u_nely", params.oper.ny) }}  ! number of elements in y direction
      {{ parameter("u_nelz", params.oper.nz) }}  ! number of elements in z direction

You can do something similar.

paugier commented 1 year ago

Yes, extending SIZE.j2 is the way to go then. @ashwinvis is there an example somewhere of how it should be done?

@akhoubani could be please prepare another PR (i'm going to close this one) focusing on this? Do not take care of the set_resources warning for now please (this will have to be done in another PR).

ashwinvis commented 1 year ago

Not yet, but hinted here

https://snek5000.readthedocs.io/en/latest/packaging.html#src-snek5000-canonical-templates-init-py

You need to copy the SIZE.j2 template, modify and use it.

paugier commented 1 year ago

It would be cleaner to use the base template and extend it. But one needs to learn a bit of jinja2.

See https://jinja.palletsprojects.com/en/3.0.x/templates/#template-inheritance

paugier commented 1 year ago

Regarding set_resources, @akhoubani you can know use instead (see https://github.com/snek5000/snek5000/pull/190)

sim.make.exec('run_fg', nproc=2)

Could you please create a PR with this change in your examples scripts?

snek5000 / snek5000-cbox

Putting back aspect ratio #30

Codecov Report