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

resource-query: Integrate resource-query into flux-sched #283

Merged
merged 30 commits into from Feb 5, 2018

Conversation

Projects
None yet
7 participants
@dongahn
Copy link
Contributor

dongahn commented Dec 1, 2017

I will keep integrating resource-query and scheduler infrastructure code into the sched. Instead of posting a giant PR with all of the commits at the end, I will use this experimental PR as a way to get incremental feedback on my integration.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Dec 1, 2017

Coverage Status

Coverage remained the same at 77.788% when pulling 58e8077 on dongahn:resource-query into 9adf92e on flux-framework:master.

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Dec 1, 2017

Codecov Report

Merging #283 into master will decrease coverage by 0.77%.
The diff coverage is 70.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #283      +/-   ##
==========================================
- Coverage   75.46%   74.69%   -0.78%     
==========================================
  Files          34       49      +15     
  Lines        7419     9080    +1661     
==========================================
+ Hits         5599     6782    +1183     
- Misses       1820     2298     +478
Impacted Files Coverage Δ
resource/dfu_traverse_impl.hpp 100% <100%> (ø)
resource/utilities/command.hpp 100% <100%> (ø)
resource/resource_gen_spec.hpp 100% <100%> (ø)
resource/dfu_match_cb.hpp 26.31% <26.31%> (ø)
resource/utilities/resource-query.cpp 38.46% <38.46%> (ø)
resource/planner/planner.c 77.85% <50%> (+1.34%) ⬆️
resource/resource_gen_spec.cpp 54.54% <54.54%> (ø)
resource/utilities/command.cpp 56.02% <56.02%> (ø)
resource/resource_graph.hpp 59.25% <59.25%> (ø)
resource/dfu_match_id_based.hpp 62% <62%> (ø)
... and 23 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9adf92e...f95b6f4. Read the comment docs.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Dec 2, 2017

Coverage Status

Coverage remained the same at 77.788% when pulling b6971c4 on dongahn:resource-query into 9adf92e on flux-framework:master.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Dec 11, 2017

Coverage Status

Coverage decreased (-0.06%) to 77.73% when pulling 68cfc72 on dongahn:resource-query into 9adf92e on flux-framework:master.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Dec 11, 2017

Coverage Status

Coverage decreased (-0.06%) to 77.73% when pulling b4f07ba on dongahn:resource-query into 9adf92e on flux-framework:master.

@SteVwonder

This comment has been minimized.

Copy link
Member

SteVwonder commented Dec 11, 2017

We should probably document any dependencies that flux-sched has (that flux-core doesn't) in the README.md. Am I correct in saying that this only adds a dependency on libboost? Is there a particular minimum version that it requires?

@dongahn

This comment has been minimized.

Copy link
Contributor Author

dongahn commented Dec 11, 2017

Good point. boost (1.53 or higher) and readline.

dongahn added some commits Dec 1, 2017

resource-query: Add README.md
Explain the overall goal of resource-query and how it is
related to our scheduling infrastructure.

Consist of five sections:

- Resource Query Utility
- Generating Resources Using GraphML (GRUG)
- Resource Selection Policy
- Fully vs. Paritially Specified Resource Request
- Limitations of Depth-First and Up (DFU) Traversal

The first commit of the resource-query PR.
build: Add support for checking boost/readline + clean up
Check for boost system, filesystem, graph libraries and readline
package that the upcoming resource-query and scheduler infrastructure
will depend on. Pull in some new m4 files.

Also add support to be able to link library dependencies with
distinct Makefile variables. Use PKG_CHECK_MODULES in
configure.ac for the packages that already support this
interface. Use AC_CHECK_LIB tricks for libraries without
such support.

Is needed as flux-sched will introduce more components each with
disparate library dependencies. Without this support, a sinlge
library dependency list exported through LIBS will be used
for all components even if some libraries in the list
are not used by certain components.
@coveralls

This comment has been minimized.

Copy link

coveralls commented Dec 15, 2017

Coverage Status

Coverage decreased (-0.06%) to 77.73% when pulling d4c2889 on dongahn:resource-query into 9adf92e on flux-framework:master.

@morrone
Copy link
Contributor

morrone left a comment

It will probably take days of additional work for me to complete a full review, but I thought I would at least submit some of my early comments right now.


typedef std::string subsystem_t;
typedef std::map<subsystem_t, std::string> multi_subsystems_t;
typedef std::map<subsystem_t, std::set<std::string> > multi_subsystemsS;

This comment has been minimized.

@morrone

morrone Dec 16, 2017

Contributor

We're using C++11 so you can do ">>" rather than "> >".

This comment has been minimized.

@dongahn

dongahn Dec 20, 2017

Author Contributor

Heh. Right, C++11 fixed this problem. Will change it as well other places where I have the old style: > >

typedef std::map<subsystem_t, std::set<std::string> > multi_subsystemsS;

struct color_t {
enum color_offset_t {

This comment has been minimized.

@morrone

morrone Dec 16, 2017

Contributor

If you make this a C++ "enum class colors: uint64_t", you won't need to do all of the casts below.

This comment has been minimized.

@dongahn

dongahn Dec 28, 2017

Author Contributor

Looks like you still needs to do static_cast<uint64_t> even with explicit enum field typing.

http://en.cppreference.com/w/cpp/language/enum

}
} // namespace fold

class scoring_api_t {

This comment has been minimized.

@morrone

morrone Dec 16, 2017

Contributor

Since this class takes several pages of screen space, I'm finding this a bit difficult to read. I think my first suggestion would be to move the definition of the class's variables up to here at the beginning of the class definition. Many of the functions use m_ssys_map, for instance, so seeing that defined up front would aid understanding.

Then again, I suppose that you are trying to keep the public interfaces listed first, and private later. That is understandable. Perhaps then the better approach would be to only declare the functions in the class definition, and use external method definitions.

This comment has been minimized.

@dongahn

dongahn Dec 20, 2017

Author Contributor

Why don't I talk to you in person about your "external method definitions" - I don't think I fully understand your suggestion.

This comment has been minimized.

@morrone

morrone Dec 21, 2017

Contributor

If you have a class named "foo", rather than defining a method "bar" inside of foo's definition, define it externally using the "foo::bar" syntax.

This comment has been minimized.

@dongahn

dongahn Dec 21, 2017

Author Contributor

Okay, I can see how this can improve readability. Will do this.

inframap m_imap;
};

typedef property_map<resource_graph_t, pinfra_t>::type vtx_infra_map_t;

This comment has been minimized.

@morrone

morrone Dec 16, 2017

Contributor

Perhaps we should consider using C++ type aliases rather than typedef?

This comment has been minimized.

@dongahn

dongahn Dec 20, 2017

Author Contributor

Oh. Let me look into that.

typedef property_map<resource_graph_t, rinfra_t>::type edg_infra_map_t;
typedef graph_traits<resource_graph_t>::vertex_descriptor vtx_t;
typedef graph_traits<resource_graph_t>::edge_descriptor edg_t;
typedef graph_traits<resource_graph_t>::vertex_iterator vtx_iterator;

This comment has been minimized.

@morrone

morrone Dec 16, 2017

Contributor

It seems to me that this is inconsistent usage of "_t" naming convention. The more that I think about it, the more that I don't see any reason to use "_t" for this kind of typedef or type alias. Of course it is a "type" of some kind, we only use it in places where types can be used. What useful information is the "_t" providing in that case?

This comment has been minimized.

@dongahn

dongahn Dec 20, 2017

Author Contributor

This came from my previous C++ projects. I prefer defining the C++ styles in our C++ coding standard first and retrofit the changes at once. (If that makes sense to you.) I'd be happy if you spearhead penciling in our C++ coding standard as well!

The issue has been open for a while: flux-framework/rfc#96

This comment has been minimized.

@morrone

morrone Dec 21, 2017

Contributor

I'd be more open to that if within one set of code it was at least internally consistent..._t on some typedefs and no _t on others doesn't seem to be terribly consistent.

This comment has been minimized.

@dongahn

dongahn Dec 21, 2017

Author Contributor

Ah... now I see your point. Yes, this makes sense. I will change vtx_iterator to vtx_iterator_t If there is other places, I will change them too. Then, when we put our C++ coding style in place, we can have a global style change pass. Thanks @morrone!

{
return c <= (color_base + (uint64_t)WHITE_OFFSET);
}
uint64_t white (uint64_t color_base) const

This comment has been minimized.

@morrone

morrone Dec 16, 2017

Contributor

In general, I don't think I am a fan of this style of function overloading. "white" and "white" here have very different operations: one compares two values, the other converts a value. I think that good overloading should limit itself to all of the same-named methods doing basically the same thing, but with different types of variables.

So the previous "bool white()" function should maybe be named "is_white()", and this one can stay "white()" or perhaps change to "to_white()".

This comment has been minimized.

@dongahn

dongahn Dec 20, 2017

Author Contributor

Was a space saver for the client code. Again I'd be happy to change it as you suggested.

This comment has been minimized.

@morrone

morrone Dec 21, 2017

Contributor

I'm not clear on how giving the functions the same name is "space saving"...

* 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
* See also: http://www.gnu.org/licenses/
\*****************************************************************************/

This comment has been minimized.

@morrone

morrone Dec 16, 2017

Contributor

I have a general question on a number of these headers: Should all of this really be header-only code? For instance, there appears to be no reason why the methods of struct color_t couldn't be implemented in a .cpp file rather than in this header. There seem to be a large number of other methods that seem equally out of place in a header.

This comment has been minimized.

@dongahn

dongahn Dec 20, 2017

Author Contributor

Good point. Some of them started as template classes and then ended up becoming plain classes. I will move some of the plain classes into implementation files.

}
~schedule_t ()
{
tags.clear ();

This comment has been minimized.

@morrone

morrone Dec 16, 2017

Contributor

Are any of these calls to .clear() actually necessary? The maps are about to have their destructors' called anyway.

planner_destroy (&x_checker);
}

std::map<int64_t, int64_t> tags;

This comment has been minimized.

@morrone

morrone Dec 16, 2017

Contributor

It doesn't look like in current usage that these need to be sorted. So maybe std::unordered_map<> would be better?

This comment has been minimized.

@dongahn

dongahn Dec 20, 2017

Author Contributor

I typically prefer std::map because it has a bit better space efficiency and traversal is deterministic (http://www.ocoudert.com/blog/2011/05/30/how-to-make-software-deterministic)

unordered_map has runtime advantage, but I am not fully convinced if that outweighs the benefits of map... Why don't I keep std::map here, and during a performance optimization phase, I will keep unordered_map as one of the potential tools.

typedef std::map<subsystem_t, std::string> multi_subsystems_t;
typedef std::map<subsystem_t, std::set<std::string> > multi_subsystemsS;

struct color_t {

This comment has been minimized.

@morrone

morrone Dec 16, 2017

Contributor

This seems like an odd use of a struct to me. It has no actual storage of its own, so I would think that at the very least it would be a class rather than a struct. And the only way it is used in the pull request is to declare an "color_t m_color;" in class dfu_impl_t. But that variable doesn't actually store anything. The real storage then happens in "uint64_t m_color_base". Maybe what you need here is an actual class that stores a color value.

This comment has been minimized.

@dongahn

dongahn Dec 20, 2017

Author Contributor

Well, I actually don't see much difference between a class and struct in C++ But if that increase your readability, I'd be happy to change it to a class.

This comment has been minimized.

@morrone

morrone Dec 21, 2017

Contributor

That is the least important of my arguments here. s/struct/class and the question is the same.

@dongahn

This comment has been minimized.

Copy link
Contributor Author

dongahn commented Dec 16, 2017

Thanks @morrone.

With all the CORAL work and debugging project work which is getting good momentum with WCI code teams, I have not been able to make as much progress as I wanted.

My current priority is to complete the first integration including working through the issues to turn Travis green. Then I will circle back to your review points and address as many as possie— Feel free to continue to have your review for the time being, though.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Dec 18, 2017

Coverage Status

Coverage decreased (-0.06%) to 77.73% when pulling 0e656ca on dongahn:resource-query into 9adf92e on flux-framework:master.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Dec 19, 2017

Coverage Status

Coverage decreased (-0.06%) to 77.73% when pulling dd9cfd9 on dongahn:resource-query into 9adf92e on flux-framework:master.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Dec 19, 2017

Coverage Status

Coverage decreased (-0.06%) to 77.73% when pulling 8e78869 on dongahn:resource-query into 9adf92e on flux-framework:master.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Dec 20, 2017

Coverage Status

Coverage decreased (-3.02%) to 74.77% when pulling d25f959 on dongahn:resource-query into 9adf92e on flux-framework:master.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Dec 20, 2017

Coverage Status

Coverage decreased (-3.02%) to 74.77% when pulling d25f959 on dongahn:resource-query into 9adf92e on flux-framework:master.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Dec 20, 2017

Coverage Status

Coverage decreased (-3.02%) to 74.77% when pulling 955a196 on dongahn:resource-query into 9adf92e on flux-framework:master.

@dongahn

This comment has been minimized.

Copy link
Contributor Author

dongahn commented Dec 21, 2017

Just FYI -- I was wondering why my attempt to add the first sharness test case for resource-query failed on a few Travis CI testing configurations. (e.g., for make distcheck).

tar: flux-sched-0.4.0-47-g253fa2c/t/data/resource/jobspecs/basics/test001.cluster1.rack1.node1.slot1.socket1.core1.yaml: file name is too long (max 99); not dumped
tar: flux-sched-0.4.0-47-g253fa2c/t/data/resource/jobspecs/basics/test002.node1.slot1.socket2.core5-gpu1-memory6.yaml: file name is too long (max 99); not dumped
tar: flux-sched-0.4.0-47-g253fa2c/t/data/resource/jobspecs/basics/test003.slot2.node1.socket2.core5-gpu1-memory8.yaml: file name is too long (max 99); not dumped
tar: flux-sched-0.4.0-47-g253fa2c/t/data/resource/jobspecs/basics/test005.slot4.node1.socket2.core18-gpu1-memory32.yaml: file name is too long (max 99); not dumped
tar: flux-sched-0.4.0-47-g253fa2c/t/data/resource/jobspecs/basics/test006.cluster1.rack1.node1.slot1.socket1.core36.yaml: file name is too long (max 99); not dumped
tar: flux-sched-0.4.0-47-g253fa2c/t/data/resource/jobspecs/basics/test007.cluster1.rack1.node1.slot1.socket1.core37.yaml: file name is too long (max 99); not dumped
tar: flux-sched-0.4.0-47-g253fa2c/t/data/resource/commands/basics/test001.100x.node1.slot1.socket2.c5-g1-m6.cmds.in: file name is too long (max 99); not dumped
tar: flux-sched-0.4.0-47-g253fa2c/t/data/resource/commands/basics/test001.3x.node1.slot1.socket2.c5-g1-m6.cmds.in: file name is too long (max 99); not dumped

Apparently there is a limit (99 characters) on the file name (I think this is actually the limit on the entire path despite the error message). I will try to fix this by shortening the jobspec file names. But this probably means, we shouldn't use directories that are too deep...

@grondo

This comment has been minimized.

Copy link
Contributor

grondo commented Dec 21, 2017

Apparently there is a limit (99 characters) on the file name (I think this is actually the limit on the entire path despite the error message). I will try to fix this by shortening the jobspec file names. But this probably means, we shouldn't use directories that are too deep...

This is only a limitation of the v7 tar format which automake uses for max portability. Since we are only interested in portability to modern OSes for now, you could look into "updating" the tar format with an automake option. (I haven't had to to do this before, so it may take some experimentation)

See the tar-ustar or tar-pax Automake options here.

@dongahn

This comment has been minimized.

Copy link
Contributor Author

dongahn commented Dec 21, 2017

@grondo: yes it makes sense. Thanks.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Dec 22, 2017

Coverage Status

Coverage decreased (-3.02%) to 74.77% when pulling ab693be on dongahn:resource-query into 9adf92e on flux-framework:master.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Dec 22, 2017

Coverage Status

Coverage decreased (-3.02%) to 74.77% when pulling afb8668 on dongahn:resource-query into 9adf92e on flux-framework:master.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Dec 22, 2017

Coverage Status

Coverage decreased (-3.02%) to 74.77% when pulling 85e2274 on dongahn:resource-query into 9adf92e on flux-framework:master.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Dec 22, 2017

Coverage Status

Coverage decreased (-3.02%) to 74.77% when pulling 85e2274 on dongahn:resource-query into 9adf92e on flux-framework:master.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Dec 22, 2017

Coverage Status

Coverage decreased (-2.9%) to 74.841% when pulling 7cd521c on dongahn:resource-query into 9adf92e on flux-framework:master.

dongahn added some commits Dec 3, 2017

resource: demo match callback implementation
Add three demo match callback classes that derive
from match_cb_t and implement high ID first,
low ID first and id-based locality-aware scheduling.
resource: DFU traverser interfaces
Perform depth-first visit on the dominant subsystem and up visit
on each and all of the auxiliary subsystems selected by the matcher
callback object (dfu_match_cb_t).

Corresponding match callback methods (whose base class
is dfu_match_t) are invoked at well-defined graph visit events.

Interfaces are mainly broken into two classes:

- dfu_traverser_t provides the public interfaces

- dfu_impl_t provides the implementation detail interfaces
used by dfu_traverser_t through an inheritance.

- Use protected inheritance so that these implementation
interfaces remain protected from external users via dfu_traverser_t.

- Mark the implementation code with "detail" namespace.
grug: Add resource generator recipe reader support
Read in a GRUG file and load a generation recipe graph
using Boost Graph Library's dynamic property support.

Designed to be used by resource generators, which use
resource_gen_spec_t class to generate a resource
graph store.
resource: Add resource generator using GRUG
Read in a GRUG file and generate resource graph data store.
resource-query: Add the initial implementation
Add a command-line utility that takes in an HPC resource
request written in Flux's Canonical Job Specification
(or simply a jobspec) (RFC 14) and selects the best-matching
compute and other resources in accordance with a selection policy.

Designed to serve as a debugging and testing interface
for our scheduling infrastructure

The main cli command is "match".

This command takes in a jobspec file name and either
allocates or reserves its best-matching resources.

Likewise, this command provides two subcommands: "allocate" will
try to allocate the best-matching resources
for the given jobspec; "allocate_orelse_reserve" will try to reserve
the resources into the future
(i.e., earliest possibly scheduable point), if an allocation
cannot be created on the current resource state.

The output format of an allocation or reservation is a reversed
tree shape where the root resource vertex appears at the last line.
This is less than ideal, but we put only a minimal effort
to the output format as the format will change in a near
future anyway: we will determine what "R" should be as
part of a co-design with flux-core's jobshell functionality.
@coveralls

This comment has been minimized.

Copy link

coveralls commented Dec 28, 2017

Coverage Status

Coverage decreased (-1.3%) to 76.519% when pulling a916317 on dongahn:resource-query into 9adf92e on flux-framework:master.

dongahn added some commits Dec 20, 2017

test: Add basic sharness tests for resource-query
Test the match capability of resource-query using
a very small machine configuration (t/data/resource/grugs/tiny.graphml)
and simple jobspec files that reside in t/data/resource/jobspecs/basics.

The text commands files are in t/data/resource/commands/basics/*
and they contain a set of match commands on these jobspec files.

Redirect each of these commands files into the resource-query utility
as input so that resource-query performs a match on each of the
match commands in the input file.

Print the best matching resources into an output file in the end,
and the output is compared against the expected output file
in t/data/resource/expected/basics.

Test both high ID first demo selection policy (-P high option
of resource-query) as well as low ID first demo policy (-P low).
test: Add resource-query tests for full vs. partial spec
Test various full and partial hierarchical specifications
using a medium sized machine configuration in
t/data/resource/grugs/medium.graphml
and jobspec files that reside in t/data/resource/jobspecs/*.

Test the correctness of resource-query for the fact that
the resource section of a jobspec can be fully
or partitially hierarchically specified. A fully specified
request describes the resource shape fully from the root
to the requested resources with respect to the resource
graph data used by resource-query.
A partially specified resource request omits the prefix
(i.e., from the root to the highest-level resources in
the request).

The text commands files are in
t/data/resource/commands/omit_prefix/* and they contain
a set of match commands on these jobspec files.

Print the best matching resources into an output file in the end,
and the output is compared against the expected output file
in t/data/resource/expected/omit_prefix.
test: Add resource-query tests for global constraints
Test global (e.g., cluster, rack etc) constraints
using a medium sized machine configuration in
t/data/resource/grugs/medium.graphml
and jobspec files that reside in
t/data/resource/jobspecs/global_constraints/*.

The text commands files are in
t/data/resource/commands/global_constraints/* and they contain
a set of match commands on these jobspec files.

Print the best matching resources into an output file in the end,
and the output is compared against the expected output file
in t/data/resource/expected/global_constraints.
test: Add resource-query tests for exclusivity
Test exclusivity at various levels using a medium sized
machine configuration in t/data/resource/grugs/medium.graphml
and jobspec files that reside
in t/data/resource/jobspecs/exclusive/*.

The text commands files are in
t/data/resource/commands/exclusive/* and they contain
a set of match commands on these jobspec files.

Print the best matching resources into an output file in the end,
and the output is compared against the expected output file
in t/data/resource/expected/exclusive.
test: Add resource-query tests for reservation
Test reservation (match allocate_orelse_reserve
using a medium sized machine configuration in
t/data/resource/grugs/resv_test.graphml
and jobspec files t/data/resource/jobspecs/reservation/*.

The text commands files are in
t/data/resource/commands/reservation/* and they contain
a set of match commands on these jobspec files.

Print the best matching resources into an output file in the end,
and the output is compared against the expected output file
in t/data/resource/expected/reservation.
test: Add resource-query tests for advanced cases
Add tests for matching local burst buffers and
heterogenous compute nodes using a machine configuration in
t/data/resource/grugs/advanced_test.graphml
and jobspec files t/data/resource/advanced/reservation/*.

There are limitations in matching for heterogenous
compute-node cases: Please refer to
"Limitations of Depth-First and Up (DFU) Traversal"
subsection of resource/utilities/README.md

The text commands files are in
t/data/resource/commands/advanced/* and they contain
a set of match commands on these jobspec files.

Print the best matching resources into an output file in the end,
and the output is compared against the expected output file
in t/data/resource/expected/advanced.
test: Add resource-query tests for IO bandwidth
Add tests for coarse grained file IO bandwidth
scheduling.

Use a machine configuration in
t/data/resource/grugs/coarse_iobw.graphml
and jobspec files t/data/resource/coarse_iobw/*.

Model the PFS1 bandwidth pool as 16 x 640MB/s and
assume that the network bandwidth to/from PFS2
is not contended.

The text commands files are in
t/data/resource/commands/coarse_iobw/* and they contain
a set of match commands on these jobspec files.

Print the best matching resources into an output file
in the end, and the output is compared against the expected
output file in t/data/resource/expected/coarse_iobw.
test: Add resource-query tests for cancel
Add tests for cancelling jobs using a machine
configuration in t/data/resource/grugs/resv_test.graphml
and jobspec files t/data/resource/cancel/*.

Make sure that the reserved or allocated resources
are correctly freed and the freed resources are
used for subsequent match requests.

The text commands files are in
t/data/resource/commands/cancel/* and they contain
a set of match commands on these jobspec files.

Print the best matching resources into an output file
in the end, and the output is compared against the expected
output file in t/data/resource/expected/cancel.
test: Add resource-query tests for min/max specification
Add tests for jobspecs that specifies minimum
and maximum requirements on various resource types.

Test all three operators: "^", "*", and "+".

Use a machine configuration in
t/data/resource/grugs/resv_test.graphml
and jobspec files t/data/resource/min_max/*.

The text commands files are in
t/data/resource/commands/min_max/* and they contain
a set of match commands on these jobspec files.

Print the best matching resources into an output file
in the end, and the output is compared against
the expected output file in
t/data/resource/expected/min_max.
test: Add resource-query tests for simple power scheduling
Add tests for matching power requirements using a machine
configuration in t/data/resource/grugs/power.graphml
and jobspec files t/data/resource/power/*.

power.graphml grug file specifies two subsystems. One
for containment and the other for power.

Use the Power-Aware (PA) selection policy (-S PA) of
resource-query to use the power subsystem as the
dominant subsystem to perform matches.

Limit test jobspecs to specify only down to the compute
node level as the fringes of the power subsystem
tree in power.graphml are at the compute node level
(associations are made between the PDU level in
the power subsystem and compute-node level in
the containment subsystem).

The text commands files are in
t/data/resource/commands/power/* and they contain
a set of match commands on these jobspec files.

Print the best matching resources into an output file
in the end, and the output is compared against the expected
output file in t/data/resource/expected/power.
@coveralls

This comment has been minimized.

Copy link

coveralls commented Dec 28, 2017

Coverage Status

Coverage decreased (-1.3%) to 76.519% when pulling f95b6f4 on dongahn:resource-query into 9adf92e on flux-framework:master.

@dongahn

This comment has been minimized.

Copy link
Contributor Author

dongahn commented Dec 28, 2017

FYI -- I was getting an error for one of my C++ codes for cppcheck in Travis CI:

cppcheck --force --inline-suppr -j 2 --std=c99 --quiet --error-exitcode=1 -i src/common/libtap .
[resource/utilities/resource-query.cpp:421]: (error) Memory is allocated but not initialized: ctx
[resource/utilities/resource-query.cpp:454]: (error) Memory is allocated but not initialized: ctx

It seems adding --std=c++11 to solve the issue. We may consider adding this to flux-core's travis CI as well...

@morrone

This comment has been minimized.

Copy link
Contributor

morrone commented Jan 22, 2018

A note on boost versions: Probably this will only work with boost 1.53 through, maybe, 1.55. At 1.56 through at least 1.58 the BGL is broken an won't compile with C++11. Unfortunately 1.58 is the standard version supplied in Ubuntu 16.04.3. I tried the latest stable boost 1.66 and that didn't go well either. Our m4 libraries don't detect 1.66 correctly.

For now I've downgraded to boost 1.53 and that is letting me at compile and run.

@morrone morrone merged commit b16e2dd into flux-framework:master Feb 5, 2018

2 of 4 checks passed

codecov/patch 70.33% of diff hit (target 75.46%)
Details
codecov/project 74.69% (-0.78%) compared to 9adf92e
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-1.3%) to 76.519%
Details
@garlick

This comment has been minimized.

Copy link
Member

garlick commented Mar 29, 2018

Anybody have the runes for downgrading libboost on Ubuntu 16.04.4 LTS?

I thought something like this would do it, but perhaps I need to add an apt source for a previous release.

$ sudo apt-get install libboost-dev=1.53
Reading package lists... Done
Building dependency tree       
Reading state information... Done
E: Version '1.53' for 'libboost-dev' was not found
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.