Skip to content

Example signal computation with custom emissivity#207

Merged
lasofivec merged 11 commits intoToFuProject:develfrom
flothesof:example_signal_with_emissivity
Nov 14, 2019
Merged

Example signal computation with custom emissivity#207
lasofivec merged 11 commits intoToFuProject:develfrom
flothesof:example_signal_with_emissivity

Conversation

@flothesof
Copy link
Copy Markdown
Contributor

As discussed with @Didou09 this is a "new" example (inspired by one of the benchmarks).

@pep8speaks
Copy link
Copy Markdown

pep8speaks commented Oct 3, 2019

Hello @flothesof! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 5:80: E501 line too long (80 > 79 characters)
Line 34:1: E302 expected 2 blank lines, found 1
Line 37:80: E501 line too long (80 > 79 characters)

Line 549:62: E231 missing whitespace after ','

Comment last updated at 2019-11-06 11:04:19 UTC

@codecov-io
Copy link
Copy Markdown

codecov-io commented Oct 4, 2019

Codecov Report

Merging #207 into devel will decrease coverage by 0.06%.
The diff coverage is 40%.

Impacted file tree graph

@@            Coverage Diff             @@
##            devel     #207      +/-   ##
==========================================
- Coverage   43.93%   43.86%   -0.07%     
==========================================
  Files          70       70              
  Lines       21288    21294       +6     
==========================================
- Hits         9352     9340      -12     
- Misses      11936    11954      +18
Impacted Files Coverage Δ
tofu/data/_plot.py 91.15% <0%> (ø) ⬆️
tofu/version.py 100% <100%> (ø) ⬆️
tofu/utils.py 48.71% <38.88%> (-0.19%) ⬇️
tofu/data/_core_new.py 13.15% <0%> (-0.47%) ⬇️
tofu/geom/_comp.py 59.87% <0%> (-0.22%) ⬇️
tofu/data/_core.py 40.9% <0%> (-0.14%) ⬇️
tofu/geom/_plot.py 65.26% <0%> (-0.11%) ⬇️
tofu/geom/_core.py 63.35% <0%> (-0.11%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 488b9f3...694f462. Read the comment docs.

@flothesof
Copy link
Copy Markdown
Contributor Author

I've fiddled a bit more with the plots and realized that I have questions about the following items:

  • what are the coordinate conventions for tofu ? (X Y Z in most cases, but how is the usual coordinate system defined?) should we make this appear somewhere in the documentation (in a page dedicated to conventions used, or alternatively in the introductory examples?)
  • I was unable to get the example working using no time-dependency (setting t=None) gave me this error, while I assumed this would work
  File "tofu\geom\_GG03.pyx", line 3057, in tofu.geom._GG03.LOS_calc_signal
    val_2d = func(pts, t=t, **fkwdargs)
ValueError: Buffer has wrong number of dimensions (expected 2, got 1)

@lasofivec
Copy link
Copy Markdown
Collaborator

lasofivec commented Oct 22, 2019

I've fiddled a bit more with the plots and realized that I have questions about the following items:

* what are the coordinate conventions for tofu ? (X Y Z in most cases, but how is the usual coordinate system defined?) should we make this appear somewhere in the documentation (in a page dedicated to conventions used, or alternatively in the introductory examples?)

Probably we should do both suggested solutions. For sure a section on "tofu's conventions" is missing in the doc.

* I was unable to get the example working using no time-dependency (setting t=None) gave me this error, while I assumed this would work
  File "tofu\geom\_GG03.pyx", line 3057, in tofu.geom._GG03.LOS_calc_signal
    val_2d = func(pts, t=t, **fkwdargs)
ValueError: Buffer has wrong number of dimensions (expected 2, got 1)

As stated in the doc, the function emissivity should always return a 2D array. I fixed that directly in your example.

tofu/tofu/geom/_GG02.pyx

Lines 2882 to 2888 in 93ae5c5

func : python function st. func(pts, t=None, vect=None) => data
with pts : ndarray (3, npts) - points where function is evaluated
vect : ndarray(3, npts) - if anisotropic signal vector of emiss.
t: ndarray(m) - times where to compute the function
returns: data : ndarray(nt,nraf) if nt = 1, the array must be 2D
values of func at pts, at given time
func is the function to be integrated along the LOS

However the tutorial is still failing, this time when plotting the signal. I opened an issue regarding this, see Issue #217

@Didou09
Copy link
Copy Markdown
Member

Didou09 commented Nov 6, 2019

Problem solved, see Issue #217
Tip though : the user needs to click once on the time axis

…custom_emissivity.py gives very different results depending on whether newcalc=True or False. False definitely seems to be the proper solution
@lasofivec
Copy link
Copy Markdown
Collaborator

This example is for the online documentation, so the figures will be non interactive.
Could you add a comment in the test with what you mean?

@Didou09
Copy link
Copy Markdown
Member

Didou09 commented Nov 6, 2019

Plotting issue solved,
But bug found in _GG => opened issue #247 for @lasofivec , top priority
Thanks @flothesof , your example helped us identity a high-priority bug
New release probable when fixed, to be discussed

@lasofivec
Copy link
Copy Markdown
Collaborator

I'm going to merge this pull request since all bugs have been corrected in branch Issue247-bisect and are independent of this branch :)

@lasofivec lasofivec merged commit cdfb00a into ToFuProject:devel Nov 14, 2019
@Didou09 Didou09 mentioned this pull request Nov 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants