-
Notifications
You must be signed in to change notification settings - Fork 3
Development #44
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
Open
tbertus
wants to merge
44
commits into
cglosser:development
Choose a base branch
from
tbertus:development
base: development
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Development #44
Changes from all commits
Commits
Show all changes
44 commits
Select commit
Hold shift + click to select a range
0cf0263
Add .travis.yml file
rayhem 52156fb
Update travis to build with cmake
rayhem 5c14ee1
Get eigen package
rayhem 0d44ffb
Remove eigen deb package; include boost dependencies
rayhem 1357e27
Manually get and build eigen package
rayhem 33fae63
Actually run the eigen getter script
rayhem d967f2a
Build Eigen with sudo
rayhem 7d9ee0f
Fix up get eigen script
rayhem 417f800
Mark as executable
rayhem a7bfe54
Get boost
rayhem 91d6b8a
Quiet logs
rayhem eda915d
Debugging get boost
rayhem 32577f8
well crap.
rayhem 28c92a0
changes
rayhem 75dce9f
formating
tbertus 2cec272
Eliminated redundant calls to bloch_rhs' evaluate during corrector
tbertus 6ba5f6f
the way to win on rainbow road
tbertus 9961c14
listing what's in the boost directory
tbertus 492cb9c
added .sh to ./bootstrap.sh in get_boost.sh
tbertus 616d85e
changed make qtest to make qtest-bin and commented out anything relat…
tbertus 102ceb1
trying to get rapidcheck to work. submodules are pretty cool
tbertus 7cbdc23
automatic boost install
tbertus 10c54d8
trying to install boost automatically
tbertus 96ebe23
other changes
tbertus 8ccb2f0
temporarily removed some unit tests
tbertus d331aff
temporarily removed some unit tests
tbertus 42db27e
installing eigen through apt
tbertus 85d3b3f
boost / eigen versions available through apt do not work well with ra…
tbertus b0af103
Merge branch 'travis' into development
tbertus 9fe8f2f
added pass/fail flag to README.md
tbertus c23b752
working on badg
tbertus f8777e0
markdown troubles
tbertus 7e5be99
try to get submodule to work with travis
tbertus afed5c6
initialize rapidcheck submodule
tbertus 17697fb
removed first_call flag in favor of two seperate functions
tbertus ca8168c
reintroducing some unit tests
tbertus 42948a6
including unit test
tbertus e74e86d
corrected submodule location
tbertus 66cc336
trying to get travis CI to work
tbertus 3bf248c
trying to get rapidcheck to work with travis on this branch
tbertus 54c8771
removed unneeded comments
tbertus 47bbd65
remove old code
tbertus 6b9308a
renamed evaluate functions of rhs and interaction classes
tbertus 4817ee2
changed past_terms_of_results to past_terms_of_convolution for pull r…
tbertus 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,3 @@ | ||
| [submodule "extra/rapidcheck"] | ||
| path = extras/rapidcheck | ||
| [submodule "rapidcheck"] | ||
| path = rapidcheck | ||
| url = https://github.com/emil-e/rapidcheck.git |
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 |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| language: cpp | ||
| compiler: | ||
| #- clang | ||
| - gcc | ||
|
|
||
| before_script: | ||
| - sudo ./install/get_boost.sh > /dev/null | ||
| - sudo ./install/get_eigen.sh > /dev/null | ||
| - mkdir build && cd build | ||
| - cmake .. | ||
|
|
||
| script: make qtest-bin && ./qtest | ||
|
|
||
| addons: | ||
| apt: | ||
| sources: | ||
| - george-edison55-precise-backports | ||
| packages: | ||
| - cmake-data | ||
| - cmake | ||
| - libfftw3-dev | ||
| #- libeigen3-dev | ||
| #- libboost-test-dev | ||
| #- libboost-program-options-dev |
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
Submodule rapidcheck
deleted from
624fe9
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 |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| #!/bin/bash | ||
| echo "Changing to tmp" | ||
| cd /tmp | ||
|
|
||
| echo "Downloading and unpacking boost" | ||
| wget https://dl.bintray.com/boostorg/release/1.68.0/source/boost_1_68_0.tar.bz2 -O boost.tar.bz2 | ||
| mkdir -p boost && tar -xvf boost.tar.bz2 -C boost --strip-components 1 > /dev/null | ||
| ls | ||
|
|
||
| echo "Building boost" | ||
| cd boost/ | ||
| ls | ||
| ./bootstrap.sh --with-libraries=program_options,test | ||
| ./b2 install |
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 |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| #!/bin/bash | ||
| cd /tmp | ||
| wget https://bitbucket.org/eigen/eigen/get/3.3.2.tar.gz -O eigen.tar.gz | ||
| mkdir -p eigen/build && tar -xvzf eigen.tar.gz -C eigen --strip-components 1 | ||
| cd eigen/build && cmake .. && make install |
Submodule rapidcheck
updated
7 files
| +4 −1 | .gitignore | |
| +2 −2 | CMakeLists.txt | |
| +5 −5 | doc/generators_ref.md | |
| +2 −3 | include/rapidcheck/Assertions.h | |
| +2 −0 | include/rapidcheck/Random.h | |
| +2 −1 | include/rapidcheck/detail/BitStream.hpp | |
| +1 −1 | include/rapidcheck/gen/Numeric.hpp |
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
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
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 |
|---|---|---|
|
|
@@ -76,18 +76,59 @@ void AIM::Farfield::propagate(const int step) | |
|
|
||
| Eigen::Map<Eigen::Array3Xcd> observers(&obs_table_(front), 3, nb); | ||
| observers = 0; | ||
| // initialize temp_observers with the correct size and zero it | ||
| temp_observers = Eigen::Array3Xcd::Zero(3, nb); | ||
|
|
||
| for(int i = 0; i < table_dimensions_[0]; ++i) { | ||
| // compute convolution up to but not including the current timestep | ||
| for(int i = 1; i < table_dimensions_[0]; ++i) { | ||
| // If (step - i) runs "off the end", just propagate src[0][...] | ||
| auto wrap = std::max(step - i, 0) % table_dimensions_[0]; | ||
|
|
||
| Eigen::Map<Eigen::ArrayXcd> prop(&propagation_table_[i][0][0][0], nb); | ||
| Eigen::Map<Eigen::Array3Xcd> src(&source_table_[wrap][0][0][0][0], 3, nb); | ||
|
|
||
| // Use broadcasting to do the x, y, and z component propagation | ||
| observers += src.rowwise() * prop.transpose(); | ||
| temp_observers += src.rowwise() * prop.transpose(); | ||
| } | ||
|
|
||
| // set observers equal to temp_observers | ||
|
Owner
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. Please don't repeat code in comments; why do you need to set |
||
| // std::cout << temp_observers << "\n\n\n\n" << std::endl; | ||
| observers += temp_observers; | ||
|
|
||
| auto wrap = std::max(step, 0) % table_dimensions_[0]; // needed ? | ||
|
|
||
| Eigen::Map<Eigen::ArrayXcd> prop(&propagation_table_[0][0][0][0], nb); | ||
| Eigen::Map<Eigen::Array3Xcd> src(&source_table_[wrap][0][0][0][0], 3, nb); | ||
| observers += src.rowwise() * prop.transpose(); | ||
|
|
||
| const auto o_ptr = &obs_table_(front); | ||
| fftw_execute_dft(spatial_vector_transforms_.backward, | ||
| reinterpret_cast<fftw_complex *>(o_ptr), | ||
| reinterpret_cast<fftw_complex *>(o_ptr)); | ||
| } | ||
|
|
||
| void AIM::Farfield::propagate_present_field(const int step) | ||
| { | ||
| const auto wrapped_step = step % table_dimensions_[0]; | ||
| const auto nb = 8 * grid->size(); | ||
| const std::array<int, 5> front = {{wrapped_step, 0, 0, 0, 0}}; | ||
|
|
||
| const auto s_ptr = &source_table_(front); | ||
| fftw_execute_dft(spatial_vector_transforms_.forward, | ||
| reinterpret_cast<fftw_complex *>(s_ptr), | ||
| reinterpret_cast<fftw_complex *>(s_ptr)); | ||
|
|
||
| Eigen::Map<Eigen::Array3Xcd> observers(&obs_table_(front), 3, nb); | ||
| observers = 0; | ||
|
|
||
| observers += temp_observers; | ||
|
|
||
| auto wrap = std::max(step, 0) % table_dimensions_[0]; // needed ? | ||
|
|
||
| Eigen::Map<Eigen::ArrayXcd> prop(&propagation_table_[0][0][0][0], nb); | ||
| Eigen::Map<Eigen::Array3Xcd> src(&source_table_[wrap][0][0][0][0], 3, nb); | ||
| observers += src.rowwise() * prop.transpose(); | ||
|
|
||
| const auto o_ptr = &obs_table_(front); | ||
| fftw_execute_dft(spatial_vector_transforms_.backward, | ||
| reinterpret_cast<fftw_complex *>(o_ptr), | ||
|
|
||
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.
Shouldn't this be
return results + past_terms_of_convolution? Ifevaluateis called, and thenevaluate_present_fieldis called,resultsgets multiple "presents" added to it, no?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.
resultsis set to zero at the top ofevaluate_present_fieldso it has the contribution of a one "present" at a time. As a matter of curiosity, because the function returns a reference, wouldreturn results + past_terms_of_convolutionwork?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.
Ah, yes, ok - that makes sense. There's still something I'm not seeing, though; ultimately I think
evaluateis really justpast_part+present_part. You've moved the logic forpresent_partinto the other function but you don't call it here so I'm not sure what your intention is.And you're exactly right: using
return results + past;returns what's called an "rvalue" (so a reference to it is invalid). Coding it that way would require changing the function signature.Uh oh!
There was an error while loading. Please reload this page.
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.
I am not sure I follow. As of now,
evaluatecomputes and stores the past and adds it to the present.evaluate_present_fieldzeros outresultscomputes the updated present contribution and adds it to thepast_partthat was calculated byevaluateUh oh!
There was an error while loading. Please reload this page.
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.
Hmm, so, in my head
evaluateshould always give you the full field (perhaps inefficiently) because it's the virtual function prescribed byInteractionBase. "An interaction can always beevaluated", so to speak, and you may not need the complexity of divorcing the past update from the present update (if, say, you're not using something as sophisticated as the predictor/corrector). So I think a good design here would be to implement two methods,ResultArray &AIM::DirectInteraction::evaluate_pastandresultArray &AIM::DirectInteraction::evaluate_present, that each populate an internal array of known size (likeresults). Then you'll have something like this:and
To do this, it may make sense to hoist
evaluate_pastandevaluate_presentintoInteractionBaseas pure virtual functions. This would be the case if everyInteractionthat ever exists must define those functions to make sense (which is probably the case).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.
I agree that would be a good way to set things up. But to be clear
evaluatecan currently be used independent ofevaluate_present_field. The functionality ofevaluatenow is the same as it was originally with the added side effect of storingpast_results.