xBOUT icon indicating copy to clipboard operation
xBOUT copied to clipboard

Some changes for FCI

Open dschwoerer opened this issue 3 years ago • 7 comments

f08d14d6ada93e1a32e084dd7a3501d07bfb0416 is not FCI related, but I hope to add soon attributes to zoidberg grids, in which case it would be great if that would be preserved ...

dschwoerer avatar Sep 30 '22 10:09 dschwoerer

Codecov Report

Merging #251 (07ebb9f) into master (280a570) will decrease coverage by 1.93%. The diff coverage is 3.15%.

:mega: This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##           master     #251      +/-   ##
==========================================
- Coverage   69.01%   67.09%   -1.93%     
==========================================
  Files          15       16       +1     
  Lines        3208     3300      +92     
  Branches      790      802      +12     
==========================================
  Hits         2214     2214              
- Misses        732      822      +90     
- Partials      262      264       +2     
Impacted Files Coverage Δ
xbout/tracing.py 0.00% <0.00%> (ø)
xbout/geometries.py 69.58% <75.00%> (-0.37%) :arrow_down:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

codecov-commenter avatar Sep 30 '22 11:09 codecov-commenter

Is it worth adding a method to BoutDataset so that if a dataset has FCI geometry it's possible to get a plot from a one-line call, something like ds.bout.poincare_plot()?

Do you know how to do that? That certainly would be nice.

It would be lovely to have a test for the field-line tracing, but I guess that could be pretty complicated to set up.

Test would be easy if we had an example grid. Can we download one for the tests? At least for CI it should be no issue. We could host it on zenodo or at the mpcdf gitlab or something else?

dschwoerer avatar Oct 27 '22 21:10 dschwoerer

For a method, in BoutDataset add something like (?)

"""
docstring...
"""
def poincare_plot(
    self,
    rz,
    yind=0,
    num=100,
    early_exit="warn",
    direction="forward",
    new_fig=True,
    show=False,
    save_as=None
):
    ds = self.data
    mytracer = Tracer(ds, direction=direction)
    rz = mytracer.poincare(rz=rz, yind=yind, num=num, early_exit=early_exit)

    if new_fig:
        plt.figure()
    p = plt.scatter(rz)
    if save_as:
        plt.savefig(save_as)
    if show:
        plt.show()

    return p

Probably need to also set some default arguments so that the plot looks nice. Maybe also have a scatter_kwargs=None argument that lets a dict be passed to tweak the plot?

johnomotani avatar Oct 28 '22 09:10 johnomotani

About the grid file for testing: Indeed don't want to add a binary file to the install, so would be a test that only runs from the source repo. We could add another directory like extra_tests so it doesn't get installed. For managing the file, we could either add it to the repo with git-lfs, or put it on Zenodo and wget it. I think git-lfs is simpler, and I guess it won't be a huge file (?) - I don't think developers would mind downloading 100MB or so...

johnomotani avatar Oct 28 '22 09:10 johnomotani

Oh sorry, I misread. I thought you wanted to only add the method if it is an fci dataset.

Concerning git-lfs, I do not like the github one, they have not only strict size limits, which should be fine, but also limits on traffic, so the fci grid repo is already putting me at the limit ... So I will go with zenodo + caching, if you do not like the mpcdf gitlab approach, which I use for xemc3 ...

dschwoerer avatar Oct 28 '22 09:10 dschwoerer

Oh sorry, I misread. I thought you wanted to only add the method if it is an fci dataset.

That would be complicated! Probably should add some check that the dataset is an fci one in the method though.

If it's not a lot more complicated, I think I'd prefer Zenodo to mpcdf-gitlab, as it's not dependent on having someone with an mpcdf account to keep it going.

johnomotani avatar Oct 28 '22 10:10 johnomotani

Still missing:

  • [ ] tests
  • [ ] dataset accessor

dschwoerer avatar Feb 02 '23 11:02 dschwoerer