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

Fix the interface of make_I #62

Merged
merged 3 commits into from
Feb 15, 2023
Merged

Fix the interface of make_I #62

merged 3 commits into from
Feb 15, 2023

Conversation

engintoklu
Copy link
Collaborator

The function evotorch.tools.make_I was accepting a single integer as the size indicator of the identity matrix. However, the method Problem.make_I was working only when the size was given as a single-element tuple. This pull request makes their interfaces compatible by making isure that both will work with a single integer or with a size tuple containing a single integer. Tests are updated to verify the correctness of the revised interfaces.

@flukeskywalker
Copy link
Contributor

👍🏽

flukeskywalker
flukeskywalker previously approved these changes Feb 5, 2023
Copy link
Collaborator

@NaturalGradient NaturalGradient left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks good but I suspect that due to this issue I had avoided using make_I in some other parts of the source. Let me check now

@codecov-commenter
Copy link

codecov-commenter commented Feb 5, 2023

Codecov Report

Merging #62 (5c7d266) into master (aa57024) will increase coverage by 0.11%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master      #62      +/-   ##
==========================================
+ Coverage   77.74%   77.86%   +0.11%     
==========================================
  Files          49       49              
  Lines        7321     7329       +8     
==========================================
+ Hits         5692     5707      +15     
+ Misses       1629     1622       -7     
Impacted Files Coverage Δ
src/evotorch/algorithms/cmaes.py 82.94% <100.00%> (ø)
src/evotorch/distributions.py 81.16% <100.00%> (-0.07%) ⬇️
src/evotorch/tools/misc.py 86.42% <100.00%> (+0.06%) ⬆️
src/evotorch/tools/tensormaker.py 94.59% <100.00%> (+6.02%) ⬆️
src/evotorch/core.py 74.55% <0.00%> (+0.07%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

pliskowski
pliskowski previously approved these changes Feb 6, 2023
Copy link
Collaborator

@pliskowski pliskowski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

engintoklu and others added 3 commits February 14, 2023 00:33
The function `evotorch.tools.make_I` was
accepting a single integer as the size
indicator of the identity matrix. However,
The method `Problem.make_I` was working only
when the size was given as a single-element
tuple. This commit makes their interfaces
compatible by making isure that both will
work with a single integer or with a size
tuple containing a single integer.
Tests are updated to verify the correctness
of the revised interfaces.
@Higgcz Higgcz merged commit 13860c7 into master Feb 15, 2023
@Higgcz Higgcz deleted the fix/make-identity branch February 15, 2023 13:47
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.

6 participants