-
Notifications
You must be signed in to change notification settings - Fork 13
Feature #252: clearer error and check for emissivity function #254
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
Merged
Merged
Changes from all commits
Commits
Show all changes
24 commits
Select commit
Hold shift + click to select a range
93f57bd
[Issue #252] added functions to check if user given is rightly defined
lasofivec 27cea6d
[Issue #252] added call to check function got rid of old param
lasofivec ae69153
[Issue #252] bf: let a semicolon by mistake
lasofivec 565d736
[Issue #252] trying something
lasofivec 136c54c
[Issue252] PEP8 Compliance
d272f2f
[Issue252] All assert if check_ff() replaced by if / raise statements
7a480fa
Merge branch 'devel' into feature-#252
lasofivec fb4b530
[Issue252] Unique full error msg in check_ff(), complemented by error…
a19ccfe
Merge branch 'feature-#252' of https://github.com/ToFuProject/tofu in…
1f49781
[Issue252] PEP8 Compliance and more robust check on ani in check_ff()
3cf115d
[#252] right definition of wrapped function
lasofivec cf892fe
[Issue252] Solved remaining issue with Rays.calc_signal(plot=True): s…
7403d10
[merge] merged with devel
lasofivec 389ed21
[merge] merged with feature 252
lasofivec 677d649
[bisect 252] undid changes in tuto
lasofivec 67fdeee
[feature] cleaner way of defining 2nd dim
lasofivec 11ab20c
[bug fix #255] found bug
lasofivec a667bec
[bf #255] added doc for ani/vect definition
lasofivec fcc95f0
[pep8] changes for flake
lasofivec 820e73f
[doc] typo corrections
lasofivec 6b2afd4
[bf #255] ani=None by default
lasofivec 8ab1c43
[bf-#255] Fixed unit tests for tutorials
8b88511
Merge pull request #257 from ToFuProject/bf-#255
Didou09 20e66de
Merge pull request #256 from ToFuProject/feature-#252-bisect
Didou09 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5760,6 +5760,100 @@ def calc_min_rho_from_Plasma2D(self, plasma, t=None, log='min', | |
| for vv in np.split(val, lind, axis=-1)]) | ||
| return vals, pts, t | ||
|
|
||
| def get_inspector(self, ff): | ||
| out = inspect.signature(ff) | ||
| pars = out.parameters.values() | ||
| na = np.sum([(pp.kind == pp.POSITIONAL_OR_KEYWORD | ||
| and pp.default is pp.empty) for pp in pars]) | ||
| kw = [pp.name for pp in pars if (pp.kind == pp.POSITIONAL_OR_KEYWORD | ||
| and pp.default is not pp.empty)] | ||
| return na, kw | ||
|
|
||
| def check_ff(self, ff, t=None, ani=None): | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All good to me, the more detailed the error messages, the better it is for everyone indeed :-) |
||
| # Initialization of function wrapper | ||
| wrapped_ff = ff | ||
|
|
||
| # Define unique error message giving all info in a concise way | ||
| # Optionnally add error-specific line afterwards | ||
| msg = ("User-defined emissivity function ff must:\n" | ||
| + "\t- be a callable (function)\n" | ||
| + "\t- take only one positional arg " | ||
| + "and at least one keyword arg:\n" | ||
| + "\t\t - ff(pts, t=None), where:\n" | ||
| + "\t\t\t - pts is a (3, npts) of (x, y, z) coordinates\n" | ||
| + "\t\t\t - t can be None / scalar / iterable of len(t) = nt\n" | ||
| + "\t- Always return a 2d (nt, npts) np.ndarray, where:\n" | ||
| + "\t\t - nt = len(t) if t is an iterable\n" | ||
| + "\t\t - nt = 1 if t is None or scalar\n" | ||
| + "\t\t - npts is the number of pts (pts.shape[1])\n\n" | ||
| + "\t- Optionally, ff can take an extra keyword arg:\n" | ||
| + "\t\t - ff(pts, vect=None, t=None), where:\n" | ||
| + "\t\t\t - vect is a (3, npts) np.ndarray\n" | ||
| + "\t\t\t - vect contains the (x, y, z) coordinates " | ||
| + "of the units vectors of the photon emission directions" | ||
| + "for each pts. Present only for anisotropic emissivity, " | ||
| + "unless specifically indicated otherwise " | ||
| + "(with ani=False in LOS_calc_signal).\n" | ||
| + "\t\t\tDoes not affect the outpout shape (still (nt, npts))") | ||
|
|
||
| # .. Checking basic definition of function .......................... | ||
| if not hasattr(ff, '__call__'): | ||
| msg += "\n\n => ff must be a callable (function)!" | ||
| raise Exception(msg) | ||
|
|
||
| npos_args, kw = self.get_inspector(ff) | ||
| if npos_args != 1: | ||
| msg += "\n\n => ff must take only 1 positional arg: ff(pts)!" | ||
| raise Exception(msg) | ||
|
|
||
| if 't' not in kw: | ||
| msg += "\n\n => ff must have kwarg 't=None' for time vector!" | ||
| raise Exception(msg) | ||
|
|
||
| # .. Checking time vector ......................................... | ||
| ltypeok = [int, float, np.int64, np.float64] | ||
| is_t_type_valid = (type(t) in ltypeok or hasattr(t, '__iter__')) | ||
| if not (t is None or is_t_type_valid): | ||
| msg += "\n\n => t must be None, scalar or iterable !" | ||
| raise Exception(msg) | ||
| nt = len(t) if hasattr(t, '__iter__') else 1 | ||
|
|
||
| # .. Test anisotropic case ....................................... | ||
| if ani is None: | ||
| is_ani = ('vect' in kw) | ||
| else: | ||
| assert isinstance(ani, bool) | ||
| is_ani = ani | ||
|
|
||
| # .. Testing outputs ............................................... | ||
| test_pts = np.array([[1, 2], [3, 4], [5, 6]]) | ||
| npts = test_pts.shape[1] | ||
| if is_ani: | ||
| vect = np.ones(test_pts.shape) | ||
| try: | ||
| out = ff(test_pts, vect=vect, t=t) | ||
| except Exception: | ||
| msg += "\n\n => ff must take ff(pts, vect=vect, t=t) !" | ||
| raise Exception(msg) | ||
| else: | ||
| try: | ||
| out = ff(test_pts, t=t) | ||
| except Exception: | ||
| msg += "\n\n => ff must take a ff(pts, t=t) !" | ||
| raise Exception(msg) | ||
|
|
||
| if not (isinstance(out, np.ndarray) and (out.shape == (nt, npts) | ||
| or out.shape == (npts,))): | ||
| msg += "\n\n => wrong output (always 2d np.ndarray) !" | ||
| raise Exception(msg) | ||
|
|
||
| if nt == 1 and out.shape == (npts,): | ||
| def wrapped_ff(*args, **kwargs): | ||
| res_ff = ff(*args, **kwargs) | ||
| return np.reshape(res_ff, (1, -1)) | ||
|
|
||
| return is_ani, wrapped_ff | ||
|
|
||
| def _calc_signal_preformat(self, ind=None, DL=None, t=None, | ||
| out=object, Brightness=True): | ||
| msg = "Arg out must be in [object,np.ndarray]" | ||
|
|
@@ -5886,7 +5980,7 @@ def calc_signal( | |
| self, | ||
| func, | ||
| t=None, | ||
| ani=False, | ||
| ani=None, | ||
| fkwdargs={}, | ||
| Brightness=True, | ||
| res=None, | ||
|
|
@@ -5922,11 +6016,6 @@ def calc_signal( | |
| => the method returns W/m2 (resp. W/m2/sr) | ||
| The line is sampled using :meth:`~tofu.geom.LOS.get_sample`, | ||
|
|
||
| The integral can be computed using three different methods: | ||
| - 'sum': A numpy.sum() on the local values (x segments lengths) | ||
| - 'simps': using :meth:`scipy.integrate.simps` | ||
| - 'romb': using :meth:`scipy.integrate.romb` | ||
|
|
||
| Except func, arguments common to :meth:`~tofu.geom.LOS.get_sample` | ||
|
|
||
| Parameters | ||
|
|
@@ -5941,6 +6030,15 @@ def calc_signal( | |
| - vect: None / (3,N) np.ndarray, unit direction vectors (X,Y,Z) | ||
| Should return at least: | ||
| - val : (N,) np.ndarray, local emissivity values | ||
| method : string, the integral can be computed using 3 different methods | ||
| - 'sum': A numpy.sum() on the local values (x segments) DEFAULT | ||
| - 'simps': using :meth:`scipy.integrate.simps` | ||
| - 'romb': using :meth:`scipy.integrate.romb` | ||
| minimize : string, method to minimize for computation optimization | ||
| - "calls": minimal number of calls to `func` (default) | ||
| - "memory": slowest method, to use only if "out of memory" error | ||
| - "hybrid": mix of before-mentioned methods. | ||
|
|
||
|
|
||
| Returns | ||
| ------- | ||
|
|
@@ -5966,6 +6064,7 @@ def calc_signal( | |
| # Launch # NB : find a way to exclude cases with DL[0,:]>=DL[1,:] !! | ||
| # Exclude Rays not seeing the plasma | ||
| if newcalc: | ||
| ani, func = self.check_ff(func, t=t, ani=ani) | ||
| s = _GG.LOS_calc_signal( | ||
| func, | ||
| Ds, | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, so I understand you are taking the function checking out of _GG to place into _core right ?
That's fine for me, just want to be sure I get it ;-)