Skip to content

Scripts for building SMLP in virtual environment#51

Open
mdmitry1 wants to merge 34 commits intomasterfrom
venv_build
Open

Scripts for building SMLP in virtual environment#51
mdmitry1 wants to merge 34 commits intomasterfrom
venv_build

Conversation

@mdmitry1
Copy link
Collaborator

Recommended for SMLP developers

@mdmitry1 mdmitry1 marked this pull request as draft February 16, 2026 19:34
Copy link
Collaborator Author

@mdmitry1 mdmitry1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added build benchmarks

@mdmitry1 mdmitry1 marked this pull request as ready for review February 17, 2026 04:19
Copy link
Collaborator Author

@mdmitry1 mdmitry1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These script enable SMLP development is virtual environment.
Validated in Ubuntu 24.04

Copy link
Collaborator Author

@mdmitry1 mdmitry1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed unnecessary temporary files

Copy link
Collaborator Author

@mdmitry1 mdmitry1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Improved Docker container usability

Copy link
Collaborator Author

@mdmitry1 mdmitry1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Viewed one more file

Copy link
Collaborator Author

@mdmitry1 mdmitry1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added boost temporary files cleanup command

Copy link
Collaborator Author

@mdmitry1 mdmitry1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added regression support

Copy link
Collaborator Author

@mdmitry1 mdmitry1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added instructions for running regression in Docker environment

Copy link
Collaborator Author

@mdmitry1 mdmitry1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switched to wheel-based build

Copy link
Collaborator Author

@mdmitry1 mdmitry1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed issues in Dockerfile and README.md

commit 1912a6e1f2cabfec6ba1f1dfde0e0b5d9b3237a2 (HEAD -> master, origin/master, origin/HEAD)
Author: Franz Brauße <dev@karlchenofhell.org>
Date:   Sat Mar 7 15:15:20 2026 +0100

    fix compilation issue for macosx-sdk 26:
Copy link
Collaborator Author

@mdmitry1 mdmitry1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rebuild wheel after commit in kay repository

Copy link
Collaborator

@fbrausse fbrausse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like having venv as a top-level directory due to clutter. As these files seem part of some specific installation instructions, which are not a hard requirement, they should be inside some build- or installation-specific directory preferrably not called venv, because that is a name users typically use to setup their own virtual environments. My suggestion would be to put all these "support scripts" into a directory called scripts instead.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would we put a binary into the source code tree of SMLP? The place for binary release files is on our releases page (currently only Github).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my point of view, this patch is adding technical debt. Let's instead fix our usage of matplotlib properly.

#!/usr/bin/tcsh -f
mkdir -p $HOME/smlp_shared
if( ! $?TZ ) setenv TZ `readlink /etc/localtime | sed 's@/usr/share/zoneinfo/@@'`
docker run -e TZ=$TZ -v $HOME/smlp_shared:/shared -it mdmitry1/smlp-dev:latest $*
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the purpose of setting TZ here? Is it a workaround of some sort? If so, could you add a comment describing what's accomplished by that?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this file?

scipy==1.11.4
seaborn
tensorflow==2.15.1
z3-solver==4.8.12
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Version pinning should be avoided as much as possible. It's not a permanent solution. For instance, we already confirmed that newer versions of tensorflow work. Why are they pinned?


#tkdiff patch for https://bugs.launchpad.net/bugs/2139062
COPY tkagg_patch.sh .
RUN chmod +x tkagg_patch.sh
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See also my comment on the patch file itself. Wouldn't it be better to ship these changes as a separate branch if they are indeed required?

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