| Time |
S |
Nick |
Message |
| 00:18 |
|
|
favonia joined #darcs |
| 00:37 |
|
|
stepcut joined #darcs |
| 01:02 |
|
|
darcscommitbot joined #darcs |
| 01:37 |
|
|
intripoon_ joined #darcs |
| 04:31 |
|
Heffalump |
morning |
| 05:31 |
|
|
bsrkaditya joined #darcs |
| 05:31 |
|
Heffalump |
morning! |
| 05:31 |
|
bsrkaditya |
Hi! |
| 05:32 |
|
Heffalump |
I completely forgot about summer time, so I'll end up being on a train and in a few tunnels for some of the time depending on how long we take |
| 05:32 |
|
Heffalump |
but they should be mostly brief interruptions |
| 05:32 |
|
bsrkaditya |
Okay |
| 05:33 |
|
Heffalump |
I had a quick look through the code and your last chat with Benedikt and I noticed a few things to focus on |
| 05:33 |
|
bsrkaditya |
What are they? |
| 05:33 |
|
Heffalump |
firstly, a minor point - your code needs to be warnings-free to go into darcs (that's been inconsistently adhered to in the past, but we're making an effort right now at least!) |
| 05:34 |
|
Heffalump |
secondly, we should discuss the [Sealed2 Patch] versus FL Patch issue. |
| 05:35 |
|
bsrkaditya |
Hmm, I know how to remove most of the warnings, but there is a problem with 1 |
| 05:35 |
|
Heffalump |
and thirdly, how visible the patch index should be to the user, which is a general UI point that should be discussed more widely, but I thought I'd just offer my perspective |
| 05:36 |
|
bsrkaditya |
match_co baling out <darcs-2.9.1:Darcs.Patch.ApplyMonad.ApplyMonadBase{tc r24hv} ( m{tv a2kvj} [tv] :: ghc-prim:GHC.Prim.*{(w) tc 34d} -> ghc-prim:GHC.Prim.*{(w) tc 34d} )> |
| 05:36 |
|
Heffalump |
ah, that's not a warning |
| 05:36 |
|
Heffalump |
you can ignore it, it's just a GHC message. I dunno why they left them in there in a release build. |
| 05:37 |
|
bsrkaditya |
Okay. I can take care of the warnings then. :-) |
| 05:38 |
|
Heffalump |
for the [Sealed2] stuff, is there a general pattern to the way that annotatePI and changesPI work that could be abstracted out? |
| 05:39 |
|
Heffalump |
I'm particularly wondering if the patch index code could work as a pre-filter that just provide the normal annotate and changes functions with a different list of patches that is smaller but produces the same result |
| 05:40 |
|
bsrkaditya |
In annotate that is what happens(mostly) |
| 05:40 |
|
bsrkaditya |
In changes, however there are many filtering functions, and they take PatchSet as input |
| 05:41 |
|
bsrkaditya |
unless we change all of them, or have the patch index filter return a patch set itself |
| 05:41 |
|
bsrkaditya |
we cannot use it as the first filter |
| 05:41 |
|
Heffalump |
is there any reason it can't do that? |
| 05:41 |
|
bsrkaditya |
Two reasons |
| 05:42 |
|
bsrkaditya |
first, changes has options like --last |
| 05:42 |
|
bsrkaditya |
which is meant to be operated on first |
| 05:42 |
|
bsrkaditya |
ie, --last is applied before filtering by file name |
| 05:43 |
|
bsrkaditya |
second, these functions take PatchSet as input, and my filtering implementation can return only [Sealed2 a] |
| 05:44 |
|
Heffalump |
(off to catch a train, back in a few minutes) |
| 05:49 |
|
Heffalump |
where's the code that does that? |
| 05:50 |
|
Heffalump |
I'm thinking that if we can find a general pattern that uses unsafeCoerce in one place to ignore patches we don't want, the filtering code can call that and more easily work on either PatchSet or FL |
| 05:55 |
|
bsrkaditya |
I do not understand. :-) |
| 05:56 |
|
Heffalump |
wher code that returns [Sealed2 a] |
| 05:56 |
|
Heffalump |
s/wher/where's the |
| 05:56 |
|
bsrkaditya |
changes and annotate use different functions |
| 05:57 |
|
bsrkaditya |
changes uses filterPatches in FileMod |
| 05:58 |
|
bsrkaditya |
anotate uses getPatches/getPatchesMultipleFiles in ShowPatchIndex |
| 05:58 |
|
bsrkaditya |
both return [Sealed2 a] |
| 05:59 |
|
bsrkaditya |
but filterPatches takes [Sealed2 a] as input |
| 05:59 |
|
Heffalump |
ok - the stuff in ShowPatchIndex should move somewhere lower-level |
| 06:00 |
|
bsrkaditya |
whereas getPatches uses readRepo (initiates the filtering) |
| 06:00 |
|
Heffalump |
what's the map (\(Sealed2 p) -> seal2 (invert p)) about? |
| 06:00 |
|
bsrkaditya |
I will move it |
| 06:00 |
|
Heffalump |
that's a bit of a smell because it suggests the code wouldn't work properly with witnesses |
| 06:00 |
|
bsrkaditya |
where is that line? |
| 06:01 |
|
bsrkaditya |
I found it |
| 06:01 |
|
bsrkaditya |
you have to invert the patches for annotate |
| 06:01 |
|
Heffalump |
how does it work in the non-patch index case? |
| 06:01 |
|
bsrkaditya |
The original implementation uses invertRL I thing |
| 06:01 |
|
bsrkaditya |
s/thing/think |
| 06:02 |
|
Heffalump |
taking a step back from the witnesses for a minute, a question about laziness occurs to me. |
| 06:02 |
|
Heffalump |
Is reading patches with the patch index filtering just as lazy as reading them without? i.e. if a command would only have read the first/last 20 patches in the repository before, is that still the case? |
| 06:03 |
|
bsrkaditya |
You mean if the annotate ended without reading the whole repository? |
| 06:04 |
|
bsrkaditya |
I never used reverse or something like that anywhere. |
| 06:05 |
|
bsrkaditya |
I can't see how I can break laziness |
| 06:05 |
|
Heffalump |
ok |
| 06:05 |
|
Heffalump |
so what I'm thinking with filtering is that you could have a function like |
| 06:06 |
|
Heffalump |
unsafeRemove :: (forall wA wB . p wA wB -> Bool) -> Either (p wA wB) (EqCheck wA wB) |
| 06:06 |
|
Heffalump |
sorry, not quite |
| 06:06 |
|
Heffalump |
perhaps |
| 06:07 |
|
Heffalump |
unsafeRemove :: (p wA wB -> Bool) -> EqCheck wA wB |
| 06:07 |
|
Heffalump |
hmm |
| 06:07 |
|
Heffalump |
not quite again :-) |
| 06:07 |
|
bsrkaditya |
what is EqCheck? |
| 06:07 |
|
Heffalump |
have a look in Darcs.Witnessses.Eq |
| 06:07 |
|
Heffalump |
the basic idea is that it has a constructor IsEq that asserts that two witnesses are equal |
| 06:08 |
|
Heffalump |
so if you make an IsEq value with unsafeCoerce, you can use it to remove a patch from a list |
| 06:08 |
|
Heffalump |
e.g. suppose you have patch1;patch2;patch3 where patch2 :: p wA wB |
| 06:09 |
|
Heffalump |
if you have a value IsEq :: EqCheck wA wB, that's an assertion that wA = wB (which is a lie) |
| 06:09 |
|
Heffalump |
so you can use it to make the witnesses list patch1;patch3 |
| 06:09 |
|
bsrkaditya |
I see. |
| 06:11 |
|
bsrkaditya |
This(converting [Sealed2 a] to PatchSet) will be useful in annotate, but not in changes |
| 06:12 |
|
bsrkaditya |
useful in annotate as we can remove the duplicated functions annotatePI and annotate' in Darcs/Annotate.hs |
| 06:12 |
|
Heffalump |
I'm proposing that where you have [Sealed2 a] now, you replace it with whatever type the existing code works on - either FL or PatchSet |
| 06:12 |
|
bsrkaditya |
but in changes the first problem (the first filter not being by file) |
| 06:13 |
|
Heffalump |
if you look in Darcs.Witnesses.Ordered, there's a function filterFLFL, which takes a function from p wX wY -> EqCheck wX wY |
| 06:13 |
|
Heffalump |
and there should either be an existing filter on patchsets that takes the same functon, or we can write one |
| 06:13 |
|
Heffalump |
so I think unsafeRemove :: (p wA wB -> Bool) -> p wA wB -> EqCheck wA wB makes sense |
| 06:14 |
|
Heffalump |
unsafeRemove f p | f p = unsafeCoerce IsEq |
| 06:14 |
|
Heffalump |
| otherwise = NotEq |
| 06:14 |
|
Heffalump |
or perhaps the other way around, depending on the sense of the function that is passed in (my definition means True for remove this patch, False for don't remove it) |
| 06:16 |
|
Heffalump |
btw, I think newset2FL does do a reverse, so I'm not quite sure about the laziness issues |
| 06:17 |
|
bsrkaditya |
Ah. Then the reading is not lazy. :-) |
| 06:18 |
|
bsrkaditya |
Still that does not mean the entire process is going to be strict. |
| 06:19 |
|
Heffalump |
if you read an entire repo where darcs didn't before, that could be a significant performance hit |
| 06:19 |
|
bsrkaditya |
Reading the repo is fast. negligible time |
| 06:19 |
|
bsrkaditya |
less that 0.1 sec |
| 06:19 |
|
Heffalump |
you should make sure to test/benchmark cases for anntoate where the file was recently added to the repo, and for changes where only the last n patches are being inspected |
| 06:19 |
|
Heffalump |
actually reading all the patch files won't be anywhere near that fast |
| 06:20 |
|
Heffalump |
I'm not even convinced that reading just the inventory files of a large repo like darcs is that fast |
| 06:20 |
|
Heffalump |
are yuo sure you didn't time just creating the lazy list, rather than actually looking into the contents? |
| 06:23 |
|
bsrkaditya |
Creating the list took negligible time. Actually looking into contents - if it means the info in PatchInfoAnd, yes this will be loaded that fast |
| 06:23 |
|
Heffalump |
ok, so that implies that reading the inventory files is fast |
| 06:23 |
|
bsrkaditya |
patch index filters just based on info in PatchInfoAnd |
| 06:25 |
|
Heffalump |
ok, that's good - perhaps laziness doesn't matter too much then |
| 06:25 |
|
bsrkaditya |
I just ran pi changes and annoate on FileMod.hs |
| 06:25 |
|
bsrkaditya |
pi changes and changes are just as fast (0.1sec) |
| 06:26 |
|
bsrkaditya |
pi annotate is 0.2 sec, when annotate is 0.1 sec |
| 06:26 |
|
Heffalump |
ok, sounds good. Are you building up a benchmark set that you rerun every so often? It'll be useful for reporting progress and final results. |
| 06:27 |
|
bsrkaditya |
I have a few shell scripts. |
| 06:27 |
|
bsrkaditya |
I am not storing any of the results though. |
| 06:28 |
|
Heffalump |
storing the intermediate results isn't that important, but having a benchmark set you can rerun so you can produce results at any given time is wuseful |
| 06:30 |
|
bsrkaditya |
What is a benchmark set exactly? |
| 06:30 |
|
Heffalump |
just a set of things to benchmark |
| 06:30 |
|
Heffalump |
e.g. the things you described above |
| 06:31 |
|
Heffalump |
but the full set would be more comprehensive, of course |
| 06:32 |
|
Heffalump |
getting back to the witnesses, do you think you would be able to refactor along the lines I suggested? i.e. have an unsafeRemove function that can be used with the patchindex, and ifne necessary add a filterPatchSet that uses it |
| 06:32 |
|
Heffalump |
and then make the patchindex just be a pre-filtering pass for both annotate and changes, and stop using Sealed2? |
| 06:32 |
|
|
schlaftier joined #darcs |
| 06:32 |
|
Heffalump |
I'm not expecting it to be trivial to get the witnesses to work properly, so do ask for help if you need it. But Sealed2 everywhere isn't really suitable for darcs. |
| 06:34 |
|
bsrkaditya |
I do not undestand how getting EqCheck helps in removing the patches. ie, after I got them what should I do? |
| 06:34 |
|
Heffalump |
have a look at filterFLFL |
| 06:34 |
|
Heffalump |
if the function it is given returns IsEq, then it drops the patch from the list |
| 06:35 |
|
Heffalump |
train has just arrived in London, I'll be back in 20 mins or so |
| 06:36 |
|
bsrkaditya |
I get it |
| 06:58 |
|
|
raichoo joined #darcs |
| 06:58 |
|
Heffalump |
back |
| 06:59 |
|
Heffalump |
the final thing I was going to discuss briefly was whether it's appropriate to expose the existence of the patch index to users at all, except perhaps for debugging purposes? |
| 07:00 |
|
Heffalump |
my feeling is that it should just be a "make darcs go faster" thing, rather than something special the users need to be aware of |
| 07:02 |
|
|
darcscommitbot joined #darcs |
| 07:05 |
|
Heffalump |
anyway, I think that would be best resolved with a general discussion on the darcs-users list |
| 07:06 |
|
bsrkaditya |
Heffalump, I agree |
| 07:07 |
|
bsrkaditya |
I too think that the existance of patch index should be invisible |
| 08:25 |
|
|
sm joined #darcs |
| 08:29 |
|
|
ocharles joined #darcs |
| 08:36 |
|
bsrkaditya |
heffalump: I finished writing the witness code for pi annotate |
| 08:36 |
|
bsrkaditya |
It passes all tests |
| 08:36 |
|
bsrkaditya |
and I could remove the duplicate annotate functions of Darcs.Patch.Annotate |
| 08:37 |
|
bsrkaditya |
http://darcsden.com/bsrkaditya[…]20626083003-ae621 |
| 08:38 |
|
bsrkaditya |
the replacement getElems function is the key |
| 08:45 |
|
|
balor joined #darcs |
| 09:08 |
|
|
owst joined #darcs |
| 09:15 |
|
|
bsrkaditya left #darcs |
| 09:58 |
|
|
delamonpansie joined #darcs |
| 12:22 |
|
|
_ilbot joined #darcs |
| 12:22 |
|
|
Topic for #darcs is now can't talk? see -> | http://wiki.darcs.net/IRC | http://darcs.net/ | darcs 2.8 is out http://wiki.darcs.net/Releases/2.8 |
| 13:02 |
|
|
darcscommitbot joined #darcs |
| 13:28 |
|
|
donri joined #darcs |
| 15:36 |
|
|
raichoo joined #darcs |
| 15:47 |
|
|
owst joined #darcs |
| 16:06 |
|
|
teratorn joined #darcs |
| 16:34 |
|
|
schlaftier joined #darcs |
| 19:02 |
|
|
darcscommitbot joined #darcs |
| 19:25 |
|
|
balor joined #darcs |
| 20:54 |
|
|
donri joined #darcs |
| 20:56 |
|
|
favonia joined #darcs |
| 20:57 |
|
|
amgarchIn9 joined #darcs |
| 23:04 |
|
|
donri_ joined #darcs |
