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

Featuring guyan reduction #6

Merged
merged 1 commit into from Aug 4, 2017
Merged

Conversation

maikkirapo
Copy link
Contributor

No description provided.

@ahojukka5
Copy link
Member

Files inside build should not be added to repository, how did you add files?

@ahojukka5
Copy link
Member

Checkout master and rebase to get newest .gitignore ignoring files inside build.

@ahojukka5
Copy link
Member

git checkout master
git pull
git checkout featuring_guyan_reduction
git rebase master

@ahojukka5
Copy link
Member

ahojukka5 commented Aug 4, 2017

To remove unnecessary files:

git rm -r build

@ahojukka5
Copy link
Member

ahojukka5 commented Aug 4, 2017

After removing unwanted files and commiting changes to repository, you may want to rebase changes, i.e. squash commits to single one, see https://www.google.fi/search?q=git+squash

Read this: https://github.com/JuliaLang/julia/blob/master/CONTRIBUTING.md

@maikkirapo maikkirapo force-pushed the featuring_guyan_reduction branch 2 times, most recently from d67e22b to 9791607 Compare August 4, 2017 14:13
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 9b554016e50f617d59ea09e53942925dbadc2cd4 on featuring_guyan_reduction into ** on master**.

@ahojukka5
Copy link
Member

Could you squash these commits also?

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 9b554016e50f617d59ea09e53942925dbadc2cd4 on featuring_guyan_reduction into 80b4f79 on master.

@ahojukka5
Copy link
Member

Notice that I have merged conflicts already, so you may need to pull changes to local computer before squashing.

# This file is a part of JuliaFEM.
# License is MIT: see https://github.com/JuliaFEM/ModelReduction.jl/blob/master/LICENSE

function guyan_reduction(K, m, s)
Copy link
Member

Choose a reason for hiding this comment

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

Add docstring, describe function arguments


using ModelReduction

K =
Copy link
Member

Choose a reason for hiding this comment

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

It's better to add data related to test inside that testset

Copy link
Member

@ahojukka5 ahojukka5 left a comment

Choose a reason for hiding this comment

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

Two small changes to code


@testset "Perform Guyan Reduction" begin
expected =
[0.25 -0.25; -0.25 0.25]
Copy link
Member

Choose a reason for hiding this comment

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

To same line

expected =
[0.25 -0.25; -0.25 0.25]
result = ModelReduction.guyan_reduction(K, m, s)
@test result ≈ expected
Copy link
Member

Choose a reason for hiding this comment

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

use isapprox instead

@maikkirapo maikkirapo force-pushed the featuring_guyan_reduction branch 2 times, most recently from 20553a2 to 9b3a58f Compare August 4, 2017 15:15
@ahojukka5 ahojukka5 force-pushed the featuring_guyan_reduction branch 3 times, most recently from f270024 to cc00572 Compare August 4, 2017 15:28
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling a5a4d60 on featuring_guyan_reduction into 80b4f79 on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling a5a4d60 on featuring_guyan_reduction into 80b4f79 on master.

@ahojukka5 ahojukka5 merged this pull request into master Aug 4, 2017
@ahojukka5 ahojukka5 deleted the featuring_guyan_reduction branch August 4, 2017 16:06
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.

None yet

3 participants