-
Notifications
You must be signed in to change notification settings - Fork 6
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
Issue17 GrayBat #58
Issue17 GrayBat #58
Conversation
@@ -0,0 +1,74 @@ | |||
/** | |||
* Copyright 2013 Erik Zenker, Carlchristian Eckert, Marius Melzer |
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.
copyright should be 2015 now
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.
done
About the CI-testing: you could edit the About your questions: How can GrayBat be integrated into haseongpu best (submodule etc.)That one is pretty difficult. As a first step, I would suggest a Is it necessary to encapsulate master and slave code into functionsMaybe not necessary, but it would make the loop-structure much more readable. Do we need non-blocking in this first version (so the master could also do computations)I don't think so, we have an issue for that (#13). The real question is: do we need to throw away all the blocking code if we want to do it properly later? Because that would be a lot of unnecessary work and we might do it non-blocking from the start Can the code readability be improved somehowWhat I like about the readability:
What could be improved:
for(Vertex v : cage.hostedVertices){
if(v == master){
// loops internally over all samples and also sends abort-messages in the end
masterFunction(v, samples);
}else{
// internally loops forever until abort-message received
slaveFunction(v);
}
} |
|
||
// Create sample indices | ||
std::vector<unsigned> samples(mesh.numberOfSamples); | ||
std::generate(samples.begin(), samples.end(), Increment(0)); |
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.
This is not needed anymore, we can use
std::iota(samples.begin(), samples.end(), 0);
(from algorithms
which does exactly the same (and does not need a functor!))
It is c++11, but that is no problem now :)
Completed to work on your comments, but to conclude this pull request I first need to transform GrayBat into a clean library stucture and add it to haseongpu as a subtree depedency. I will also try to fix issue #9 by grouping all interface data into some ExperimentDescription type Furthermore, GrayBat is able to solve issue #13. I need to further investigate if this issue is totally covered by the current source state. With GrayBat, issue #12 would become obsolet, since MPI code is then not needed anymore ! |
so many nice things!
|
Oh yeah I like them too 🎈 |
🐳 |
A lot of commits later:
|
a(we)s(om)e |
# The first commit's message is: New parallel mode: Graybat (issue ComputationalRadiationPhysics#17) * Abstraction from communication method * Equal behavior than MPI mode * Can be choosen by parallelMode=graybat * Added as subtree in include/graybat The calcPhiAse interface was reduced 4 structs (issue ComputationalRadiationPhysics#9) The threaded code was updated to C++11 threads # This is the 2nd commit message: threading code now C++11 # This is the 3rd commit message: Finished graybat hook
git-subtree-dir: include/graybat git-subtree-split: 13c673e720668804c50381e0fa6811e0b1cd490f
69c597e
to
185d6a0
Compare
Git subtree and rebase are no good friends, so I will leave it with this five commits, but this is ne main commit ...we om ? |
My first thoughts for now:
There will be a more detailed review of code lines tomorrow. Probably also a full simulation with MATLAB to see if results are correct :) |
Unfortunately, testing on Hypnos found some problems and/or smaller annoyances build + environmentThis is probably more a list of TODO stuff (might need an issue)
setenv('LD_LIBRARY_PATH', ':/opt/pkg/devel/boost/1.56.0/gnu/4.8.2/64/opt/lib:/opt/pkg/mpi/openmpi/1.7.4/gnu/4.8.2/64/opt/lib:/opt/pkg/infiniband/psm/3.1/lib64:/opt/pkg/devel/cuda/6.5/lib64:/opt/pkg/devel/cuda/6.5/lib:/opt/pkg/numlib/mpfr/3.1.2/lib:/opt/pkg/numlib/mpc/1.0.1/lib:/opt/pkg/numlib/gmp/5.1.1/lib:/opt/torque/lib'); codeThis is a more serious list of problems
|
Excessive testing is finished.
|
Could you please document your test results? |
I will gladly do so. Is there a preferred form of documentation? |
I think posting those numbers here would be o.k., but in addition link to the code & version that produced these results |
The following tests were used to verify that the introduction of GrayBat didn't introduce any new value errors. SetupThe baseline used plain MPI for communication and the previous dev tip (268b6cd). The new code used GrayBat as introduced by this pull request (9734a6d). The programs were both executing the MATLAB example (in the respective commits, see the folder Each of the examples was launched on the Hypnos cluster, using a submit-file of the following form: #!/bin/bash
#PBS -q k20
#PBS -l nodes=2:ppn=8
#PBS -l walltime=96:00:00
#PBS -N HASE_GB_integration_testing
## ENVIRONMENT ############################
. /opt/modules-3.2.6/Modules/3.2.6/init/bash
export MODULES_NO_OUTPUT=1
module load ~/own.modules.kepler
export -n MODULES_NO_OUTPUT
uname -a
echo " "
cd ~/haseongpu_integration_GrayBat
matlab -r laserPumpCladdingExample Run with MPI:
Run with GrayBat
ResultsTo obtain gain values for a plot of gain over time in the context of the experiment that was simulated, the values in the table below need to be processed with The results of both runs are in strong agreement with the experimental measurements.
|
A first try to use GrayBat as framework for communication in
haseongpu. The lines of code were reduced in comparison
to the mpi case and I think it is more intuitive. Some
code review would be nice, regarding:
IMPORTANT: This will not compile, since, GrayBat needs to be clone into the include directory:
cd include
git clone -b topic-haseongpu https://github.com/erikzenker/GrayBat.git