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

ENH: ignore unsatified operations with partial inputs #18

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

ankostis
Copy link

@ankostis ankostis commented Sep 29, 2019

Reason:

The purpose is not to require a "perfect" input model for the dag to run,
but the algorithm to find the minimum viable subgraph that can compute.

Useful also when 2 (or more) operations provide the same output,
and provided inputs fully satisfy only one of operation.

Changes

For instance, the graph below with this PR can go ahead and compute

  • ab_plus_c=21 when given just a=10 & b1=2, and
  • ab_plus_c=6 when given just a=10 & b2=2:

t

Before this PR, the above graph would fail on execution time while trying to evaluate needlesly
the un-satisfied operations.

NOTE that it is an invasive change and have repercussions to many decision of this library.


I confirm that this contribution is made under the terms of the license found in the root directory of this repository's source tree and that I have the authority necessary to make this contribution on behalf of its copyright owner.

ankostis added a commit to ankostis/graphtik that referenced this pull request Sep 29, 2019
Usefull when 2 (or more) operations provifing the same output,
and only one has fully satisfied inputs.
Before it would fail trying to evaluate the un-satisfied ones.

+ New TC added.
.
ankostis added a commit to ankostis/graphtik that referenced this pull request Sep 29, 2019
Usefull when 2 (or more) operations provifing the same output,
and only one has fully satisfied inputs.
Before it would fail trying to evaluate the un-satisfied ones.

+ New TC added.
.
ankostis added a commit to ankostis/graphtik that referenced this pull request Sep 29, 2019
Usefull when 2 (or more) operations provifing the same output,
and only one has fully satisfied inputs.
Before it would fail trying to evaluate the un-satisfied ones.

+ New TC added.
.
@codecov-io
Copy link

codecov-io commented Sep 29, 2019

Codecov Report

Merging #18 into master will increase coverage by 1.14%.
The diff coverage is 88.37%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #18      +/-   ##
==========================================
+ Coverage   77.87%   79.01%   +1.14%     
==========================================
  Files           5        5              
  Lines         348      367      +19     
==========================================
+ Hits          271      290      +19     
  Misses         77       77
Impacted Files Coverage Δ
graphkit/functional.py 93.82% <100%> (+0.07%) ⬆️
graphkit/network.py 72.68% <86.48%> (+2.48%) ⬆️

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 e70718b...7b99b16. Read the comment docs.

were writing in text-mode in PY3. and failing as encoding error.
ankostis added a commit to ankostis/graphtik that referenced this pull request Sep 30, 2019
+ The x2 TCs added just before are now passing.
receiving partial inputs, needed for other operations.
+ The x2 TCs added just before are now passing.
@ankostis ankostis force-pushed the enh-ignore_unsatified branch 2 times, most recently from 10c1d7f to 66e0a8c Compare October 1, 2019 09:46
NOTE dict are not deterministic in <PY3.6.
So this commit would not improve determinism
in those pythons.

+ build: add `boltons` dependency for ndexedSet.
+ doc: mark all set usage if affect determinism.
+ e.g. see reproducibility problem in yahoo#14.
@ankostis
Copy link
Author

ankostis commented Oct 1, 2019

Merged with #23 bc ordered-set is needed to rewrite unsatisfied traversal with topo-sort (without visited nodes).

ankostis referenced this pull request in syamajala/graphkit Oct 8, 2019
@ankostis
Copy link
Author

ankostis commented Oct 8, 2019

$ date
Tue 08 Oct 2019 06:55:39 PM EEST
$ cloc --by-file-by-lang  graphkit 
       5 text files.
       5 unique files.                              
       0 files ignored.

github.com/AlDanial/cloc v 1.82  T=0.01 s (539.2 files/s, 106761.0 lines/s)
------------------------------------------------------------------------------------
File                                  blank        comment           code
------------------------------------------------------------------------------------
graphkit/network.py                     116            190            237
graphkit/functional.py                   51             77             89
graphkit/base.py                         34             78             68
graphkit/__init__.py                      3              3              5
graphkit/modifiers.py                     8             29              2
------------------------------------------------------------------------------------
SUM:                                    212            377            401
------------------------------------------------------------------------------------

-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Python                           5            212            377            401
-------------------------------------------------------------------------------
SUM:                             5            212            377            401
-------------------------------------------------------------------------------

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.

2 participants