unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
From: Christopher Rodriguez <yewscion@gmail.com>
To: Liliana Marie Prikler <liliana.prikler@ist.tugraz.at>
Cc: Christopher Rodriguez <yewscion@gmail.com>, 58100@debbugs.gnu.org
Subject: [bug#58100] [PATCH] gnu: cbqn: factor out singeli into derivative package.
Date: Tue, 27 Sep 2022 13:27:23 -0400	[thread overview]
Message-ID: <87r0zwhez2.fsf@gmail.com> (raw)
In-Reply-To: <982732ff807d152d4e8c181e3c33ca03ea72ac4b.camel@ist.tugraz.at>

[-- Attachment #1: Type: text/plain, Size: 5123 bytes --]


Liliana Marie Prikler <liliana.prikler@ist.tugraz.at> writes:

> This is not how to write a ChangeLog.  See [1] or infer from other
> commits.

Thank You for pointing me in the right direction; I will ensure a
standardized changelog in my revised patch after reading all of the
linked info.

>> -         (commit "9c1cbdc99863b1da0116df61cd832137b196dc5c"))
>> +         (commit "bd823839feaf42af4013e5a245981f58f563e659"))
> Why are you changing the bootstrap commit?  This looks suspicious.

I am bumping the commit from 9c1cbdc99863b1da0116df61cd832137b196dc5c to
46501ac819c8f21c69d7d2ba4b0457a7356f5e42 (another commit was made when
closing the above-linked issue) as the new commit is the most recent one
for this un-tagged project.

There is no 'release' to package, and so in order to stay up to date
regular updates of this package's commit will likely be necessary.

I can see why the bootstrap commit might stay the same, however. Would
it be better to solely amend the commit on the actual installable
package instead?

>>      (arguments
>> -     (list #:make-flags '(list "shared-o3" "o3n-singeli")
>> +     (list #:make-flags '(list "shared-o3" "o3")
> Okay

I have actually just re-read the documentation after the abovementioned
most recent commit, and noticed a surviving line in the upstream
`README.md`[1]:

`make PIE=""` on ARM CPUs (incl. Android & M1)

Perhaps I can incorporate this into the package to allow it to build on
aarch64 as well as x86_64? Though as mentioned in the original upstream
issue, I think there is a problem with a dependent package… perhaps this
should be saved for a separate issue, then.

> Instead of providing a singeli variant, would just tuning the package
> suffice?
This was actually my biggest question when making the patch. I chose a
singeli variant because there is no architecture detection at all in the
makefile; it relies entirely on the specified targets ("o3", "c", etc)
to decide what to build for.

As an example: Switching the target from "o3n-singeli" to "o3"
immediately changed the entire build, preventing it from looking at all
for Singeli sources, even though I had yet to unlink them from the
source directory.

Is there a preferred method for this kind of build structure in a Guix
package? I suppose I could do a (cond *) in the make flags… maybe
referencing a variable for the target? I don't know what variable that
might be, though, as we would not only be looking for an x86_64 target,
but specifically that the underlying system supports AVX2…

In general, I would much prefer to keep it as one package. I think it is
much easier to maintain that way, and makes the user experience much
easier as well. But I'm sorry to report that I'm unaware of how best to
implement this, and would greatly appreciate some advice.

> If not, I think we should try to properly unbundle singeli (as in
> build an actual singeli package) before adding another package
> variant.  Then, you could use existing patterns to decide whether to
> use singeli by making it an input or not.

As for unbundling singeli: Running singeli requires a version of cbqn
built with or without singeli support. Building a version of cbqn with
singeli support requires the /source/ for singeli to be present in the
build directory at build time, not a precompiled binary. Singeli itself
is actually just a BQN script[2], and not a compiled binary at all, and
is a transpiler from BQN to IR/C. It's used in the optimized version to
transpile/compile the SIMD algorithms (sic, I am unfamiliar with this
concept).

In short, to unbundle singeli, we can just avoid including singeli in
the build, as in the revised cbqn package in the patch. We could make a
package for singeli that uses an installed bqn binary from any cbqn
package, but we would still need the sources present at build time due
to the way they are used and called in the build script. Fully
decoupling the optimized cbqn from singeli would require rewriting the
parts of the build that locate and run the singeli script, and (I think)
is more suited to an upstream patch than a package definition. I have
opened an issue regarding this possibility with upstream as of this
email[3].

(FWIW, I've also opened an issue regarding release tagging[4], though
from my previous correspondence with upstream I'm fairly certain a
release tag is not yet something they are comfortable with).

>
> Cheers

Thank You (as always) for Your guidance, Liliana. Sorry for these
issues, I am still learning best practices and standards for this
project and those of GNU in general. I hope You have a great day!

[1]: https://github.com/dzaima/CBQN/commit/46501ac819c8f21c69d7d2ba4b0457a7356f5e42
[2]: https://github.com/mlochbaum/Singeli/blob/master/singeli
[3]: https://github.com/dzaima/CBQN/issues/46
[4]: https://github.com/dzaima/CBQN/issues/47

-- 
Christopher Rodriguez
()  ascii ribbon campaign - against html e-mail
/\  www.asciiribbon.org   - against proprietary attachments

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

  reply	other threads:[~2022-09-27 18:04 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-26 19:03 [bug#58100] cbqn currently targets AVX2 Christopher Rodriguez
2022-09-26 19:12 ` [bug#58100] [PATCH] gnu: cbqn: factor out singeli into derivative package Christopher Rodriguez
2022-09-26 19:17   ` Christopher Rodriguez
2022-09-27  7:26   ` Liliana Marie Prikler
2022-09-27 17:27     ` Christopher Rodriguez [this message]
2022-09-28  7:41       ` Liliana Marie Prikler
2022-10-22 18:23 ` [bug#58100] [PATCH v2 1/8] cbqn: Build without singeli Liliana Marie Prikler
2022-11-06 17:39   ` [bug#58100] cbqn currently targets AVX2 Ludovic Courtès
2022-11-06 18:25     ` Liliana Marie Prikler
2022-11-07 13:35       ` Ludovic Courtès
2022-11-07 21:03         ` bug#58100: " Liliana Marie Prikler
2022-10-22 18:25 ` [bug#58100] [PATCH v2 2/8] gnu: Remove singeli-sources Liliana Marie Prikler
2022-10-22 18:28 ` [bug#58100] [PATCH v2 3/8] gnu: cbqn-bootstrap: Use let-bound revision Liliana Marie Prikler
2022-10-22 18:34 ` [bug#58100] [PATCH v2 4/8] gnu: cbqn: Rewrite in terms of cbqn-bootstrap Liliana Marie Prikler
2022-10-22 18:43 ` [bug#58100] [PATCH v2 5/8] gnu: cbqn: Build using GCC Liliana Marie Prikler
2022-10-22 18:59 ` [bug#58100] [PATCH v2 6/8] gnu: dbqn: Install regular files rather than copying them recursively Liliana Marie Prikler
2022-10-22 19:01 ` [bug#58100] [PATCH v2 8/8] gnu: cbqn: " Liliana Marie Prikler
2022-10-22 19:01 ` [bug#58100] [PATCH v2 7/8] gnu: cbqn-bootstrap: " Liliana Marie Prikler

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://guix.gnu.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87r0zwhez2.fsf@gmail.com \
    --to=yewscion@gmail.com \
    --cc=58100@debbugs.gnu.org \
    --cc=liliana.prikler@ist.tugraz.at \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).