Skip to content

Update base image to python-3.11.3 #455

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

Open
wants to merge 30 commits into
base: main
Choose a base branch
from
Open

Update base image to python-3.11.3 #455

wants to merge 30 commits into from

Conversation

danielhollas
Copy link
Contributor

@danielhollas danielhollas commented May 14, 2024

Closes #265

NOTE: Upgrading Python is a breaking change since the python dependencies of installed AiiDAlab Apps need to be reinstalled due, see #317

@danielhollas danielhollas added the blocked This issue/PR is blocked by another issue/PR. label Jun 21, 2024
@danielhollas
Copy link
Contributor Author

Blocked on aiidalab/aiidalab#428

@unkcpz
Copy link
Member

unkcpz commented Jun 21, 2024

Hi @danielhollas, I reconsidered if use jupyter stack as the base image is a good option, instead if we move to using aiida-core images could gaining more flexibility. Major two reasons are:

  1. The jupyter/minimal-notebook we use now as you did in this PR, we using python version tag, therefore has no control on the notebook version. If fact the python version is not we really care but mostly the aiida-core version and jupyter backend version.
  2. We cannot set the system username which is minor but still not so good. Plus we can not using s6-overlay which I think manage well for the services when we need more.

Let me know your opinion.

@danielhollas
Copy link
Contributor Author

Blocked on aiidalab/aiidalab#429

@unkcpz I've created #471, we shouldn't be discussing these foundational thing in PRs.

@unkcpz
Copy link
Member

unkcpz commented Jun 22, 2024 via email

@danielhollas
Copy link
Contributor Author

Yeah, if you want to start to play with NB7, you can start with this PR and set python=3.11.4 which is the first version with v7.

@danielhollas danielhollas changed the title Update base image to python-3.10.11 Update base image to python-3.11.3 Jun 22, 2024
@danielhollas danielhollas removed the blocked This issue/PR is blocked by another issue/PR. label Jul 14, 2024
mamba install --yes \
aiida-core==${AIIDA_VERSION} \
mamba-bash-completion \
traitlets=5.9.0 \
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The base image already comes with traitlets==5.9.0

@danielhollas danielhollas marked this pull request as ready for review September 8, 2024 13:36
@danielhollas
Copy link
Contributor Author

This is now ready for review and testing. I don't want to merge yet, since we should deploy this change together with #483 and #482 so that we bundle all possibly breaking changes in single release.

Copy link
Member

@yakutovicha yakutovicha left a comment

Choose a reason for hiding this comment

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

Thanks, @danielhollas. I just tried to run the container on my laptop, but it failed. My aiidalab-launch config looks as follows:

port = 8892
default_apps = []
system_user = "jovyan"
image = "ghcr.io/aiidalab/full-stack:pr-455"
home_mount = "aiidalab_test-python-311_home"
extra_mounts = []

Here is the log containing the error:

...


+ _log '/usr/local/bin/start.sh: running script /usr/local/bin/before-notebook.d/20_start-postgresql.sh'
+ [[ /usr/local/bin/start.sh: running script /usr/local/bin/before-notebook.d/20_start-postgresql.sh == \E\R\R\O\R\:* ]]
+ [[ /usr/local/bin/start.sh: running script /usr/local/bin/before-notebook.d/20_start-postgresql.sh == \W\A\R\N\I\N\G\:* ]]
+ [[ '' == '' ]]
+ echo '/usr/local/bin/start.sh: running script /usr/local/bin/before-notebook.d/20_start-postgresql.sh'
+ source /usr/local/bin/before-notebook.d/20_start-postgresql.sh
/usr/local/bin/start.sh: running script /usr/local/bin/before-notebook.d/20_start-postgresql.sh
++ set -x
++ [[ -z /home/jovyan/.postgresql ]]
++ PSQL_LOGFILE=/home/jovyan/.postgresql/logfile
++ PSQL_START_CMD='pg_ctl --timeout=180 -w -l /home/jovyan/.postgresql/logfile start'
++ MAMBA_RUN='mamba run -n aiida-core-services'
++ [[ ! -d /home/jovyan/.postgresql ]]
++ chmod -R g-rwxs /home/jovyan/.postgresql
++ [[ -f /home/jovyan/.postgresql/logfile ]]
++ rm -f /home/jovyan/.postgresql/logfile.1.gz
++ mv /home/jovyan/.postgresql/logfile /home/jovyan/.postgresql/logfile.1
++ gzip /home/jovyan/.postgresql/logfile.1
++ rm -vf /home/jovyan/.postgresql/postmaster.pid
removed '/home/jovyan/.postgresql/postmaster.pid'
++ mamba run -n aiida-core-services pg_ctl --timeout=180 -w -l /home/jovyan/.postgresql/logfile start
pg_ctl: could not start server
Examine the log output.

ERROR conda.cli.main_run:execute(125): `conda run pg_ctl --timeout=180 -w -l /home/jovyan/.postgresql/logfile start` failed. (See above for error)
waiting for server to start.... stopped waiting

For your information: I have ARM architecture, but I doubt it is related.

@danielhollas
Copy link
Contributor Author

@yakutovicha strange, it works for me. Have you pulled the latest version of the image? Can you dig up the postgres logs to see why it is failing?

@danielhollas
Copy link
Contributor Author

I just enabled the arm tests on this branch and they all passed.

@yakutovicha
Copy link
Member

@danielhollas, let me know how you want to proceed here. Would you like me to try it again?

@danielhollas
Copy link
Contributor Author

@danielhollas, let me know how you want to proceed here. Would you like me to try it again?

Yes, that would be great! Since I've released 2.2.3 with Python 3.11 support it should allow you to test the EMPA apps hopefully (that was the blocker we hit at the hackathon I believe).

@yakutovicha yakutovicha self-requested a review February 10, 2025 15:50
Copy link
Member

@yakutovicha yakutovicha left a comment

Choose a reason for hiding this comment

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

It is strange. Sometimes things are slow to load, sometimes things start to hang, and sometimes the notebook is loaded with some widgets missing. It feels buggy.

I have the old version running on the same machine side-by-side, and I don't see those issues there.

Maybe, a solution here could be switching to the newest 3.11.11 version I propose below?

An example, this hangs forever:

image

@@ -1,7 +1,7 @@
{
"variable": {
"PYTHON_VERSION": {
"default": "3.9.13"
"default": "3.11.3"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"default": "3.11.3"
"default": "3.11.11"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately we can't easily update since those images have updated Jupiter stack with notebook v7, with which we are not compatible.

Copy link
Member

Choose a reason for hiding this comment

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

Let's then go to 3.10 instead. We can further update to notebook v7 and fix the remaining things afterwards.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you open a different PR for it so we can test it?

Copy link
Member

Choose a reason for hiding this comment

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

sure

@danielhollas
Copy link
Contributor Author

I tried and saw the issue as well, here are some interesting errors/ warnings that I see in the JS console

This page uses the non standard property “zoom”. Consider using calc() in the relevant property values, or using “transform” along with “transform-origin: 0 0”. start.ipynb
Uncaught TypeError: dontShowId is null
    <anonymous> http://localhost:8889/apps/apps/home/start.ipynb:685
start.ipynb:685:5
    <anonymous> http://localhost:8889/apps/apps/home/start.ipynb:685
Adding a listener for DOMNodeInserted is deprecated and will be removed soon. Instead of a MutationEvent, use MutationObserver. https://developer.mozilla.org/docs/Web/API/MutationObserver jquery.min.js:2
actions jupyter-notebook:find-and-replace does not exist, still binding it in case it will be defined later... menubar.js:283
accessing "actions" on the global IPython/Jupyter is not recommended. Pass it to your objects constructors at creation time main.js:218
Failed to fetch ipywidgets through the "jupyter.widget.control" comm channel, fallback to fetching individual model state. Reason: Control comm was closed too early manager-base.js:365:32

image

If I go to the debugger I see this strange error that prompts to reload the page

image

@danielhollas
Copy link
Contributor Author

danielhollas commented Feb 17, 2025

Looking at top in the container the python process takes about 2% CPU. Given the errors above, I suspect that the problem is not really with python version.

EDIT: Huh, after reloading AtmoSpec, I got this weird TraitError

Details
---------------------------------------------------------------------------
TraitError                                Traceback (most recent call last)
/tmp/ipykernel_6049/3783618230.py in <cell line: 0>()
      9 WORKCHAIN_LABEL = "ATMOSPEC workflow"
     10 
---> 11 structure_manager_widget = TrajectoryManagerWidget(
     12     importers=[
     13         ConformerSmilesWidget(title="SMILES"),

~/.local/lib/python3.11/site-packages/aiidalab_ispg/app/widgets.py in __init__(self, importers, viewer, editors, storable, node_class, **kwargs)
    394         ]
    395 
--> 396         super(ipw.VBox, self).__init__(children=[*children, self.output], **kwargs)
    397 
    398     def _convert_to_structure_node(self, structure):

/opt/conda/lib/python3.11/site-packages/ipywidgets/widgets/widget_box.py in __init__(self, children, **kwargs)
     62     def __init__(self, children=(), **kwargs):
     63         kwargs['children'] = children
---> 64         super(Box, self).__init__(**kwargs)
     65         self.on_displayed(Box._fire_children_displayed)
     66 

/opt/conda/lib/python3.11/site-packages/ipywidgets/widgets/widget.py in __init__(self, **kwargs)
    475         """Public constructor"""
    476         self._model_id = kwargs.pop('model_id', None)
--> 477         super(Widget, self).__init__(**kwargs)
    478 
    479         Widget._call_widget_constructed(self)

/opt/conda/lib/python3.11/site-packages/traitlets/traitlets.py in __init__(self, *args, **kwargs)
   1345             for key, value in kwargs.items():
   1346                 if self.has_trait(key):
-> 1347                     setattr(self, key, value)
   1348                     changes[key] = Bunch(
   1349                         name=key,

/opt/conda/lib/python3.11/site-packages/traitlets/traitlets.py in __set__(self, obj, value)
    730             raise TraitError('The "%s" trait is read-only.' % self.name)
    731         else:
--> 732             self.set(obj, value)
    733 
    734     def _validate(self, obj, value):

/opt/conda/lib/python3.11/site-packages/traitlets/traitlets.py in set(self, obj, value)
    704 
    705     def set(self, obj, value):
--> 706         new_value = self._validate(obj, value)
    707         try:
    708             old_value = obj._trait_values[self.name]

/opt/conda/lib/python3.11/site-packages/traitlets/traitlets.py in _validate(self, obj, value)
    736             return value
    737         if hasattr(self, "validate"):
--> 738             value = self.validate(obj, value)
    739         if obj._cross_validation_lock is False:
    740             value = self._cross_validate(obj, value)

/opt/conda/lib/python3.11/site-packages/traitlets/traitlets.py in validate(self, obj, value)
   2869             return value
   2870 
-> 2871         value = self.validate_elements(obj, value)
   2872 
   2873         return value

/opt/conda/lib/python3.11/site-packages/traitlets/traitlets.py in validate_elements(self, obj, value)
   2881                 v = self._trait._validate(obj, v)
   2882             except TraitError as error:
-> 2883                 self.error(obj, v, error)
   2884             else:
   2885                 validated.append(v)

/opt/conda/lib/python3.11/site-packages/traitlets/traitlets.py in error(self, obj, value, error, info)
    821                         ),
    822                     )
--> 823             raise error
    824         else:
    825             # this trait caused an error

/opt/conda/lib/python3.11/site-packages/traitlets/traitlets.py in validate_elements(self, obj, value)
   2879         for v in value:
   2880             try:
-> 2881                 v = self._trait._validate(obj, v)
   2882             except TraitError as error:
   2883                 self.error(obj, v, error)

/opt/conda/lib/python3.11/site-packages/traitlets/traitlets.py in _validate(self, obj, value)
    736             return value
    737         if hasattr(self, "validate"):
--> 738             value = self.validate(obj, value)
    739         if obj._cross_validation_lock is False:
    740             value = self._cross_validate(obj, value)

/opt/conda/lib/python3.11/site-packages/traitlets/traitlets.py in validate(self, obj, value)
   2149             return value
   2150         else:
-> 2151             self.error(obj, value)
   2152 
   2153     def info(self):

/opt/conda/lib/python3.11/site-packages/traitlets/traitlets.py in error(self, obj, value, error, info)
    826             if self.name is None:
    827                 # this is not the root trait
--> 828                 raise TraitError(value, info or self.info(), self)
    829             else:
    830                 # this is the root trait

TraitError: The 'children' trait of a TrajectoryManagerWidget instance contains an Instance of a TypedTuple which expected a Widget, not the list [Tab(children=(ConformerSmilesWidget(children=(HBox(children=(Text(value='', placeholder='C=C'), Button(button_style='primary', description='Generate molecule', style=ButtonStyle(), tooltip='Generate molecule from SMILES string'))), HTML(value=''))), StructureUploadWidget(children=(FileUpload(value={}, description='Upload Structure', layout=Layout(width='initial')), HTML(value='<a href="https://wiki.fysik.dtu.dk/ase/ase/io/io.html#ase.io.write" target="_blank">\n        Supported structure formats\n        </a>'), StatusHTML(value=''))), StructureBrowserWidget(children=(VBox(children=(VBox(children=(HTML(value='<p>Select the date range:</p>'), HBox(children=(Text(value='2025-02-10', description='From: ', style=DescriptionStyle(description_width='120px')), Text(value='2025-02-18', description='To: '), Button(button_style='info', description='Search', layout=Layout(margin='2px 0 0 2em', width='initial'), style=ButtonStyle())))), layout=Layout(border='1px solid #fafafa', padding='1em')), HTML(value='<hr>'), HBox(children=(RadioButtons(layout=Layout(width='25%'), options=('all', 'uploaded', 'edited', 'calculated'), value='all'), Dropdown(description='Process Label', disabled=True, layout=Layout(width='50%'), options=('All',), style=DescriptionStyle(description_width='120px'), value='All'))))), HTML(value='<hr>'), Dropdown(layout=Layout(width='900px'), options=(('Select a Structure (0 found)', False),), value=False)))), _titles={'0': 'SMILES', '1': 'Upload file', '2': 'AiiDA database'})].

This is with traitlets=5.9.0 and ipywidgets=7.7.4
List of commits between ipywidgets 7.7.4 and 7.8.5: jupyter-widgets/ipywidgets@7.7.4...7.8.5
We really need to investigate more #512

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support Python 3.11
3 participants