Skip to content
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

models: change defaults for Box1D parameters #1854

Merged
merged 1 commit into from Aug 24, 2023

Conversation

DougBurke
Copy link
Contributor

@DougBurke DougBurke commented Aug 21, 2023

Summary

The xlow parameter of Box1D now defaults to 1 and the ampl parameter has a greatly-increased range (from the previous version of -1 to 1).

Details

This changes the Box1D parameters so that

  • xhi now starts at 1 rather than 0
  • ampl uses the same range as xlow/xhi rather than -1 to +1

The CIAO 4.15 behaviour:

>>> print(Box1D())
box1d
   Param        Type          Value          Min          Max      Units
   -----        ----          -----          ---          ---      -----
   box1d.xlow   thawed            0 -3.40282e+38  3.40282e+38
   box1d.xhi    thawed            0 -3.40282e+38  3.40282e+38
   box1d.ampl   thawed            1           -1            1

The new behaviour:

>>> print(Box1D())
box1d
   Param        Type          Value          Min          Max      Units
   -----        ----          -----          ---          ---      -----
   box1d.xlow   thawed            0 -3.40282e+38  3.40282e+38
   box1d.xhi    thawed            1 -3.40282e+38  3.40282e+38
   box1d.ampl   thawed            1 -3.40282e+38  3.40282e+38

Fix #1265

This changes the Box1D parameters so that

- xhi now starts at 1 rather than 0
- ampl uses the same range as xlow/xhi rather than -1 to +1

>>> print(Box1D())
box1d
   Param        Type          Value          Min          Max      Units
   -----        ----          -----          ---          ---      -----
   box1d.xlow   thawed            0 -3.40282e+38  3.40282e+38
   box1d.xhi    thawed            0 -3.40282e+38  3.40282e+38
   box1d.ampl   thawed            1           -1            1

to

>>> print(Box1D())
box1d
   Param        Type          Value          Min          Max      Units
   -----        ----          -----          ---          ---      -----
   box1d.xlow   thawed            0 -3.40282e+38  3.40282e+38
   box1d.xhi    thawed            1 -3.40282e+38  3.40282e+38
   box1d.ampl   thawed            1 -3.40282e+38  3.40282e+38

Fix sherpa#1265
@DougBurke DougBurke added type:enhancement note: easy review Indicates PR requires simple review labels Aug 21, 2023
@codecov
Copy link

codecov bot commented Aug 21, 2023

Codecov Report

Merging #1854 (d78a659) into main (0b8b18c) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1854   +/-   ##
=======================================
  Coverage   81.24%   81.24%           
=======================================
  Files          73       73           
  Lines       26452    26452           
  Branches     3959     3959           
=======================================
  Hits        21491    21491           
  Misses       4800     4800           
  Partials      161      161           
Files Changed Coverage Δ
sherpa/models/basic.py 80.98% <100.00%> (ø)

Continue to review full report in Codecov by Sentry.

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

@wmclaugh wmclaugh merged commit 1e30628 into sherpa:main Aug 24, 2023
18 checks passed
@DougBurke DougBurke deleted the fix_1265 branch August 24, 2023 12:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
note: easy review Indicates PR requires simple review type:enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Default parameter range for Box1D.ampl seems restrictive
3 participants