mumps 5.6.2 (new formula) #154418

johnwparent · 2023-11-15T17:49:01Z

Currently the only tap that I could find vendoring MUMPS is no longer maintained and is out of date. Packages currently in brew/core actually depend on MUMPS and vendor the package as a resource rather than a package in its own right, which has resulted in some users installing things like ipopt simply to get the mumps headers/etc.
MUMPS should be a first class package.

Have you followed the guidelines for contributing?
Have you ensured that your commits follow the commit style guide?
Have you checked that there aren't other open pull requests for the same formula update/change?
Have you built your formula locally with HOMEBREW_NO_INSTALL_FROM_API=1 brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
Does your build pass brew audit --strict <formula> (after doing HOMEBREW_NO_INSTALL_FROM_API=1 brew install --build-from-source <formula>)? If this is a new formula, does it pass brew audit --new <formula>?

github-actions · 2023-11-15T17:53:25Z

Thanks for contributing to Homebrew! 🎉 It looks like you're having trouble with a CI failure. See our contribution guide for help. You may be most interested in the section on dealing with CI failures. You can find the CI logs in the Checks tab of your pull request.

Formula/m/mumps.rb

SMillerDev · 2023-11-15T20:31:09Z

Formula/m/mumps.rb

+    gcc_major_ver = Formula["gcc"].any_installed_version.major
+    optf << "-fallow-argument-mismatch" if gcc_major_ver >= 10


The current one is 13, so this doesn't have to be a conditional

Forgive me, I'm not overly familiar with homebrew, but is there no scenario where a user could have an older GCC? Or is that a scenario that's just not covered?

You're depending on gcc, so it'll never be lower than it is now (gcc 13)

Makes sense, thanks! Was not aware that's how homebrew resolved deps.

Formula/m/mumps.rb

johnwparent · 2023-12-05T22:02:05Z

@SMillerDev I have addressed most comments.

The remaining point of uncertainty from the previous review is the "magic". I've removed all that I can while still allowing MUMPS to build for me. I've reached out to the MUMPS folks to see about upstreaming the install target, but AFAICT they have no public VCS so I just have the patch locally. I don't foresee the quick installation logic I have here presenting much of a maintenance issue, and on the off chance MUMPS modifies their installation tree in a way the recipe doesn't handle, it's a quick update. Maintaining the patch arbitrarily seems much harder and I'd rather not hold off this MR on a mailing list response.

All other build system intervention is fairly site-specific (or are specific instructions from the MUMPS build docs) and as such can't really be upstreamed in a generic fashion.

Formula/i/ipopt.rb

fahasch · 2023-12-18T13:21:50Z

The errors in the compilation might be due to the fact that the present formula compiles with clang. Given the fact that it is a (mixture) of fortran and c, compiled with gfortran, the gcc compiler might be more appropriate.

johnwparent · 2023-12-18T20:10:31Z

The errors in the compilation might be due to the fact that the present formula compiles with clang. Given the fact that it is a (mixture) of fortran and c, compiled with gfortran, the gcc compiler might be more appropriate.

@fahasch Thanks for the insight, I was under the impression that this stanza: depends_on "gcc" would enforce that gcc is the compiler, rather than clang. AFAIK mumps cannot be compiled with the clang compiler (even with flang) so this is likely the cause of the issue as you said.

fahasch · 2023-12-18T20:32:03Z

I'm not completely sure: it is true that ENV.cc should be gcc. To be absolutely sure you can add fails_with :clang after depends_on gcc.

I just tried your formula locally (on MacOS). It turns out that the problem is with the linker. The option -soname is not supported by ld. It should be replaced by -install_name. The relevant parameter might be SONAME. You find some ideas how to organize the linker in the formula for scotch.

github-actions · 2024-01-09T00:02:29Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

Formula/m/mumps.rb

johnwparent · 2024-01-16T17:47:51Z

@fahasch I believe I have made all requested edits and the package seems to be working now.

fahasch · 2024-01-16T17:55:20Z

Agreed! Let us see what @SMillerDev thinks about the formula.

Formula/m/mumps.rb

SMillerDev · 2024-01-17T09:15:26Z

Formula/m/mumps.rb

+    inreplace "Makefile.inc", ".so", ".dylib" unless OS.linux?
+    inreplace "Makefile.inc", "# RPATH_OPT = -Wl,-rpath,/path/to/MUMPS_x.y.z/lib/", "RPATH_OPT = -Wl,-rpath,#{lib}"
+    make_args = ["CDEFS=-DAdd_", "OPTF=-fallow-argument-mismatch"]
+    make_args += ["CC=#{ENV["CC"]} -fPIC",


Suggested change

make_args += ["CC=#{ENV["CC"]} -fPIC",

make_args += ["CC=#{ENV.cc} -fPIC",

johnwparent · 2024-02-07T15:15:10Z

@SMillerDev I believe I have either responded to or addressed your concerns/comments and this PR is ready for another round of review feedback.

johnwparent · 2024-02-20T16:17:50Z

@SMillerDev @fahasch Just wanted to circle back on this to keep things moving forward, please let me know what I can do to promote the progressions of this MR.

SMillerDev · 2024-02-20T16:20:02Z

Formula/m/mumps.rb

+  license "CECILL-C"
+
+  # Core dependencies
+  depends_on "gcc"


Does it depend on GCC at runtime?

No, how do I express a build/link only dep?

depends_on "gcc" => :build

SMillerDev · 2024-02-20T16:20:56Z

Formula/m/mumps.rb

+    inreplace "Makefile.inc", "-soname", "-install_name" unless OS.linux?
+    inreplace "Makefile.inc", ".so", ".dylib" unless OS.linux?
+    inreplace "Makefile.inc", "# RPATH_OPT = -Wl,-rpath,/path/to/MUMPS_x.y.z/lib/", "RPATH_OPT = -Wl,-rpath,#{lib}"


Have these issues been reported/fixed upstream?

I am discussing ln:16 and ln:17 with the MUMPS developers, ln:18 is not an issue, it's an expected and documented part of the MUMPS build workflow if you want to build w/ RPATHS.

it's an expected and documented part of the MUMPS build workflow if you want to build w/ RPATHS.

Can you link to the documentation then here please?

Suggested change

inreplace "Makefile.inc", "-soname", "-install_name" unless OS.linux?

inreplace "Makefile.inc", ".so", ".dylib" unless OS.linux?

inreplace "Makefile.inc", "# RPATH_OPT = -Wl,-rpath,/path/to/MUMPS_x.y.z/lib/", "RPATH_OPT = -Wl,-rpath,#{lib}"

# Required to build with RPATH:

# https://some.li/nk

inreplace "Makefile.inc", "-soname", "-install_name" unless OS.linux?

inreplace "Makefile.inc", ".so", ".dylib" unless OS.linux?

inreplace "Makefile.inc", "# RPATH_OPT = -Wl,-rpath,/path/to/MUMPS_x.y.z/lib/", "RPATH_OPT = -Wl,-rpath,#{lib}"

SMillerDev · 2024-02-20T16:21:36Z

Formula/m/mumps.rb

+  end
+
+  test do
+    cd testpath do


Tests are always in the testpath

🤦‍♂️ That makes a lot of sense. Will remove.

SMillerDev · 2024-02-20T16:23:41Z

Formula/m/mumps.rb

+    make_args = ["CDEFS=-DAdd_", "OPTF=-fallow-argument-mismatch"]
+    make_args += ["CC=#{ENV.cc} -fPIC",
+                  "FC=gfortran -fPIC -fopenmp",
+                  "FL=gfortran -fPIC -fopenmp"]
+    # Default lib args
+    # Note: MUMPS link cannot find LAPACK without these
+    # lines
+    blas_lib = "-L#{Formula["openblas"].opt_lib} -lopenblas"
+    make_args << "LIBBLAS=#{blas_lib}"
+    make_args << "LAPACK=#{blas_lib}"


Can you add a comment with the upstream issue report for this? Because it shouldn't need this much custom configuration to compile right?

I find it pretty common that makefile systems rely on non-trivial user intervention to correctly setup the make variables for a given build environment. MUMPS seems to take that approach.
The OPTF and CDEFS args are actually specifically required by the MUMPS build instructions docs, as are the blas and lapack specs and most of the extra compile flags. From the MUMPS build docs: "in most cases, Makefile.inc should be adapted to fit with your architecture, libraries and compilers" . I absolutely agree there are improvements than can/should be made to the MUMPS build system, and I have reached out to the MUMPS devs.

I have reached out to the MUMPS devs about an install target, and a MacOS specific makefile, as well as a few of the places I do think we should not have to specify a make argument. However the MUMPS developers do not maintain as public facing issue tracker, VCS, or archive of any kind AFAICT (from googling and some communication on the general mailing list), so there unfortunately is nothing for you folks to monitor, track, or reference. I am communicating with them through their dev mailing list, and general mailing list, which is also how I will submit patches if and when there are any. I understand you would like to avoid the added maintenance of arguments specified to a build system, so how would you like to proceed here given this context?
To be clear, I am happy to upstream or iterate on whatever is definitively necessary to upstream to make this PR work for homebrew, but with no tracking, I think perhaps this PR can move in tandem with my upstreams to MUMPS.
Maybe there's someone at homebrew I can CC on some of the MUMPS threads?

Additionally, all that said, the lack of -lblas in Make.inc/Makefile.G95.SEQ is almost certainly a bug, and I will include that in my iterations with the MUMPS devs.

I find it pretty common that makefile systems rely on non-trivial user intervention to correctly setup the make variables for a given build environment

No, it's really not that common if you look at the software inside of Homebrew.

SMillerDev · 2024-02-20T16:24:08Z

Formula/m/mumps.rb

+    # Makefile provides no install target, perform as needed install
+    # Install libraries
+    lib.install Dir["lib/#{shared_library("*")}"]
+    lib.install Dir["libseq/libmpiseq#{shared_library("*")}"]
+    # Install headers
+    libexec.install "include"
+    (libexec/"include").install Dir["libseq/*.h"]
+    include.install_symlink Dir[libexec/"include/*"]
+    # Install docs and examples
+    doc.install Dir["doc/*.pdf"]
+    pkgshare.install "examples"


Can we submit a patch upstream that adds an install target?

I am discussing this with the MUMPS developers on their dev mailing list. AFAICT (and from a response on the general mailing list) there are 0 public facing resources/trackers for MUMPS development, and changes are submitted via that same mailing list, so there is nothing for homebrew to track/reference that would have any meaning.

johnwparent · 2024-03-04T23:18:04Z

@SMillerDev Apologies for the delay, I've been OOO for the past week.
I have addressed the requested changes unrelated to upstreaming the customizations I make in this MR and believe this is ready for another round of review.
Regarding the upstreaming of these build system modifications, the MUMPS developers are interested in changes (within reason + good practice) that help them better package MUMPS for brew and to have outright build support for MacOS. I am iterating with them on how best to go about this and what these changes will look like now. However, this still leaves the issue of the lack of upstream issue/MR tracking.
My hope is that we can merge this MR as is (assuming no more code review is necessary) without needing to rely on an upstream issue to link to. In lieu of an upstream link, I propose a detailed docstring as part of this package recipe with a procedure for updating the MUMPS version that includes verifying the inclusion of my customizations and removing the salient components of the ruby code. I am however, open to really any solution that moves this along and sidesteps the issue of an upstream link, as that does not seem feasible at this time, so please let me know what the brew folks would feel most comfortable with, and we can iterate from there.
Thanks for all the review so far!
FWIW I'm also willing to be on the hook to update the progress of the upstream patches in the MUMPS brew recipe as I go.

SMillerDev · 2024-03-05T08:45:50Z

If there is a mailing list archive link, I think that would be good to add as reference. Otherwise this conversation will have to do.

johnwparent · 2024-03-05T15:39:09Z

If there is a mailing list archive link, I think that would be good to add as reference. Otherwise this conversation will have to do.

@SMillerDev I've been looking for an archive for a while now to no results. There's clearly an option to access the archive in the MUMPS mailing list signup UI, but the option appears non functional/enabled.
https://listes.ens-lyon.fr/sympa/subscribe/mumps-users for context, you can see the unhighlighted option for "archive" in the table to the left of the page.

At this point, it seems like this conversation is all we'll have to document this issue, so LGTM?

jwnimmer-tri · 2024-03-13T02:00:19Z

I'd like to help push this over the finish line, if I can. What work still remains here before this can merge?

github-actions · 2024-04-03T07:42:41Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

johnwparent · 2024-04-03T20:55:43Z

@SMillerDev Last I understood these changes were ready for the upstream a few weeks ago, are there remaining blockers? What can we do to move this PR along and prevent stale-ness?
Any help is appreciated, and thanks for your review so far!

Currently the only tap vendoring MUMPS is no longer maintained and is out of date. Packages currently in brew/core actually depend on MUMPS and vendor the package as a resource rather than a package in its own right, which has resulted in some users installing things like ipopt simply to get the mumps headers/etc. MUMPS should be a first class package. Add first class MUMPS support via a new formula for MUMPS 5.6.2

MUMPS has been added as a first class package from its actual source repository. Ipopt should depend on that rather than bringing in MUMPS as a resource from a third party repo.

MikeMcQuaid

I'm willing to change my mind here but: vendoring looks like a much better option here:

the building and installation of this software is a mess
it doesn't work with Clang (the default macOS compiler for an age)
it seems extremely likely to break or have essential parts missed in future upgrades

Sorry @johnwparent, I appreciate all the effort here and am willing to change my mind here if there's enough documentation added and linked to that this all becomes obvious but, as-is, this seems like a lot of work to maintain and much more messy than the previous vendoring approach.

MikeMcQuaid · 2024-04-05T07:33:45Z

Formula/m/mumps.rb

+  # Core dependencies
+  depends_on "gcc" => :build
+  depends_on "openblas"
+  fails_with :clang


Does this fail always and unconditionally with Clang? Why is that?

MikeMcQuaid · 2024-04-05T07:34:38Z

Formula/m/mumps.rb

+    inreplace "Makefile.inc", "-soname", "-install_name" unless OS.linux?
+    inreplace "Makefile.inc", ".so", ".dylib" unless OS.linux?
+    inreplace "Makefile.inc", "# RPATH_OPT = -Wl,-rpath,/path/to/MUMPS_x.y.z/lib/", "RPATH_OPT = -Wl,-rpath,#{lib}"


it's an expected and documented part of the MUMPS build workflow if you want to build w/ RPATHS.

Can you link to the documentation then here please?

Suggested change

inreplace "Makefile.inc", "-soname", "-install_name" unless OS.linux?

inreplace "Makefile.inc", ".so", ".dylib" unless OS.linux?

inreplace "Makefile.inc", "# RPATH_OPT = -Wl,-rpath,/path/to/MUMPS_x.y.z/lib/", "RPATH_OPT = -Wl,-rpath,#{lib}"

# Required to build with RPATH:

# https://some.li/nk

inreplace "Makefile.inc", "-soname", "-install_name" unless OS.linux?

inreplace "Makefile.inc", ".so", ".dylib" unless OS.linux?

inreplace "Makefile.inc", "# RPATH_OPT = -Wl,-rpath,/path/to/MUMPS_x.y.z/lib/", "RPATH_OPT = -Wl,-rpath,#{lib}"

MikeMcQuaid · 2024-04-05T07:35:15Z

Formula/m/mumps.rb

+    make_args = ["CDEFS=-DAdd_", "OPTF=-fallow-argument-mismatch"]
+    make_args += ["CC=#{ENV.cc} -fPIC",
+                  "FC=gfortran -fPIC -fopenmp",
+                  "FL=gfortran -fPIC -fopenmp"]
+    # Default lib args
+    # Note: MUMPS link cannot find LAPACK without these
+    # lines
+    blas_lib = "-L#{Formula["openblas"].opt_lib} -lopenblas"
+    make_args << "LIBBLAS=#{blas_lib}"
+    make_args << "LAPACK=#{blas_lib}"


I find it pretty common that makefile systems rely on non-trivial user intervention to correctly setup the make variables for a given build environment

No, it's really not that common if you look at the software inside of Homebrew.

ProfFan · 2024-04-16T18:11:02Z

It does work with clang, compiling MUMPS on my machine with Clang absolutely works.

Also it seems that all these environment modifications are not necessary - here is my Makefile.inc:

SCOTCHDIR  = $(shell brew --prefix scotch)
ISCOTCH    = -I$(SCOTCHDIR)/include
LSCOTCH    = -L$(SCOTCHDIR)/lib -lptesmumps -lscotch -lptscotch -lptscotcherr -lptscotcherrexit -lscotcherrexit

LPORDDIR = $(topdir)/PORD/lib/
IPORD    = -I$(topdir)/PORD/include/
LPORD    = -L$(LPORDDIR) -lpord

LMETISDIR = $(shell brew --prefix metis)
IMETIS    = -I$(LMETISDIR)/include

LMETIS    = -L$(LMETISDIR)/lib -lmetis

ORDERINGSF = -Dscotch -Dmetis -Dpord -Dptscotch
ORDERINGSC  = $(ORDERINGSF)

LORDERINGS = $(LMETIS) $(LPORD) $(LSCOTCH)
IORDERINGSF = $(ISCOTCH)
IORDERINGSC = $(IMETIS) $(IPORD) $(ISCOTCH)

# PLAT : use it to add a default suffix to the generated libraries
PLAT    = 
# Library extension, + C and Fortran "-o" option
# may be different under Windows
LIBEXT  = .a
OUTC    = -o 
OUTF    = -o 
# RM : remove files
RM      = /bin/rm -f
# CC : C compiler
CC      = cc
# FC : Fortran 90 compiler
FC      = mpif90
# FL : Fortran linker
FL      = mpif90
# AR : Archive object in a library
#      keep a space at the end if options have to be separated from lib name
AR      = ar vr 
# RANLIB : generate index of an archive file
#   (optionnal use "RANLIB = echo" in case of problem)
RANLIB  = ranlib
#RANLIB  = echo

# DEFINE HERE YOUR LAPACK LIBRARY
LAPACK = -llapack

# SCALAP should define the SCALAPACK and  BLACS libraries.
SCALAP  = -L$(shell brew --prefix scalapack)/lib -lscalapack

# INCLUDE DIRECTORY FOR MPI
INCPAR  = -I$(shell brew --prefix open-mpi)/include

# LIBRARIES USED BY THE PARALLEL VERSION OF MUMPS: $(SCALAP) and MPI
LIBPAR  = $(SCALAP) $(LAPACK) -L/usr/lib -lmpi

# The parallel version is not concerned by the next two lines.
# They are related to the sequential library provided by MUMPS,
# to use instead of ScaLAPACK and MPI.
INCSEQ  = -I$(topdir)/libseq
LIBSEQ  = $(LAPACK) -L$(topdir)/libseq -lmpiseq

# DEFINE HERE YOUR BLAS LIBRARY
LIBBLAS = -lblas

# DEFINE HERE YOUR PTHREAD LIBRARY
LIBOTHERS = -lpthread

# FORTRAN/C COMPATIBILITY:
#  Use:
#    -DAdd_ if your Fortran compiler adds an underscore at the end
#              of symbols,
#     -DAdd__ if your Fortran compiler adds 2 underscores,
#
#     -DUPPER if your Fortran compiler uses uppercase symbols
#
#     leave empty if your Fortran compiler does not change the symbols.
#

CDEFS = -DAdd_

#COMPILER OPTIONS
OPTF    = -O -fallow-argument-mismatch
OPTC    = -O -I.
OPTL    = -O

# CHOOSE BETWEEN USING THE SEQUENTIAL OR THE PARALLEL VERSION.

#Sequential:
#INCS = $(INCSEQ)
#LIBS = $(LIBSEQ)
#LIBSEQNEEDED = libseqneeded

#Parallel:
INCS = $(INCPAR)
LIBS = $(LIBPAR)
LIBSEQNEEDED =

Works within a regular shell and just make all.

ProfFan · 2024-04-16T18:12:38Z

The RPATH issue can be solved with install_name_tool after libs are built, btw.

github-actions bot added the autosquash Automatically squash pull request commits according to Homebrew style. label Nov 15, 2023

johnwparent mentioned this pull request Nov 15, 2023

Mumps in Homebrew RobotLocomotion/drake#20429

Open

chenrui333 changed the title ~~Add MUMPS Formulae~~ mumps 5.6.2 (new formula) Nov 15, 2023

SMillerDev reviewed Nov 15, 2023

View reviewed changes

fahasch reviewed Dec 9, 2023

View reviewed changes

Formula/i/ipopt.rb Show resolved Hide resolved

github-actions bot added stale No recent activity and removed stale No recent activity labels Jan 9, 2024

johnwparent force-pushed the add-mumps branch from 41516a7 to 2b6c84d Compare January 11, 2024 18:31

github-actions bot added automerge-skip `brew pr-automerge` will skip this pull request and removed autosquash Automatically squash pull request commits according to Homebrew style. labels Jan 11, 2024

johnwparent force-pushed the add-mumps branch from 2b6c84d to 9c61851 Compare January 11, 2024 19:16

github-actions bot removed the automerge-skip `brew pr-automerge` will skip this pull request label Jan 11, 2024

fahasch reviewed Jan 11, 2024

View reviewed changes

Formula/m/mumps.rb Show resolved Hide resolved

johnwparent force-pushed the add-mumps branch 2 times, most recently from cc52899 to 4671ad3 Compare January 11, 2024 22:51

SMillerDev reviewed Jan 17, 2024

View reviewed changes

johnwparent force-pushed the add-mumps branch from 4671ad3 to 9b681f4 Compare January 24, 2024 00:39

SMillerDev reviewed Feb 20, 2024

View reviewed changes

johnwparent force-pushed the add-mumps branch 2 times, most recently from 1703368 to 2b5b418 Compare March 4, 2024 23:12

github-actions bot added the stale No recent activity label Apr 3, 2024

johnwparent force-pushed the add-mumps branch from 2b5b418 to 0ee7793 Compare April 3, 2024 20:52

johnwparent requested a review from SMillerDev April 3, 2024 20:56

johnwparent added 2 commits April 4, 2024 15:12

ipopt: fix outdate mumps usage

271f013

MUMPS has been added as a first class package from its actual source repository. Ipopt should depend on that rather than bringing in MUMPS as a resource from a third party repo.

johnwparent force-pushed the add-mumps branch from 0ee7793 to 271f013 Compare April 4, 2024 19:12

SMillerDev requested a review from a team April 4, 2024 20:16

MikeMcQuaid requested changes Apr 5, 2024

View reviewed changes

SMillerDev removed their request for review April 11, 2024 09:13

		gcc_major_ver = Formula["gcc"].any_installed_version.major
		optf << "-fallow-argument-mismatch" if gcc_major_ver >= 10

	make_args += ["CC=#{ENV["CC"]} -fPIC",
	make_args += ["CC=#{ENV.cc} -fPIC",

mumps 5.6.2 (new formula) #154418

Are you sure you want to change the base?

mumps 5.6.2 (new formula) #154418

Conversation

johnwparent commented Nov 15, 2023 • edited

github-actions bot commented Nov 15, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnwparent Nov 15, 2023 • edited

Choose a reason for hiding this comment

johnwparent commented Dec 5, 2023

fahasch commented Dec 18, 2023

johnwparent commented Dec 18, 2023

fahasch commented Dec 18, 2023

github-actions bot commented Jan 9, 2024

johnwparent commented Jan 16, 2024

fahasch commented Jan 16, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnwparent commented Feb 7, 2024

johnwparent commented Feb 20, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnwparent Feb 21, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnwparent commented Mar 4, 2024

SMillerDev commented Mar 5, 2024

johnwparent commented Mar 5, 2024

jwnimmer-tri commented Mar 13, 2024

github-actions bot commented Apr 3, 2024

johnwparent commented Apr 3, 2024

MikeMcQuaid left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ProfFan commented Apr 16, 2024

ProfFan commented Apr 16, 2024

johnwparent commented Nov 15, 2023 •

edited

johnwparent Nov 15, 2023 •

edited

johnwparent Feb 21, 2024 •

edited