Skip to content
This repository was archived by the owner on Oct 11, 2021. It is now read-only.

refactor: rename parameters to parameter_defaults#531

Merged
redeboer merged 3 commits intomasterfrom
parameter_default
Mar 24, 2021
Merged

refactor: rename parameters to parameter_defaults#531
redeboer merged 3 commits intomasterfrom
parameter_default

Conversation

@redeboer
Copy link
Copy Markdown
Member

This naming makes more sense, as HelicityModel.parameters sounds like some sequence that lists the parameters in the model, while it's actually a mapping of those parameters to default ('suggested') values.

@redeboer redeboer added the ⚠️ Interface Changes to the interface label Mar 24, 2021
@redeboer redeboer requested a review from spflueger March 24, 2021 11:47
@redeboer redeboer self-assigned this Mar 24, 2021
@redeboer redeboer added this to the Release 0.7.1 milestone Mar 24, 2021
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 24, 2021

Codecov Report

Merging #531 (2d9cdfb) into master (d5eceff) will increase coverage by 0.01%.
The diff coverage is 76.92%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #531      +/-   ##
==========================================
+ Coverage   86.10%   86.11%   +0.01%     
==========================================
  Files          22       22              
  Lines        3584     3587       +3     
  Branches      876      876              
==========================================
+ Hits         3086     3089       +3     
  Misses        319      319              
  Partials      179      179              
Flag Coverage Δ
unittests 86.11% <76.92%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/expertsystem/amplitude/helicity.py 85.66% <76.92%> (+0.13%) ⬆️

@redeboer redeboer force-pushed the parameter_default branch from 4ddce6c to 9b415d9 Compare March 24, 2021 14:04
@redeboer redeboer force-pushed the parameter_default branch from 9b415d9 to 2d9cdfb Compare March 24, 2021 15:55
@spflueger
Copy link
Copy Markdown
Member

spflueger commented Mar 24, 2021

Sounds reasonable here. Mostly because these parameters are not connected to the model. Only if you do some action like subs. I want to point out though, that in the tensorwaves Model interface, the parameters are actually connected to the model. So there the name defaults would be wrong.

Also I should add, the crucial information of that dictionary is the names of the parameters. So that the distinction between parameters and variables of the model can be made. The default values are more some convenience feature

@redeboer redeboer merged commit e4f1cbb into master Mar 24, 2021
@redeboer redeboer deleted the parameter_default branch March 24, 2021 17:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

⚠️ Interface Changes to the interface

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants