Which VM flavor to use for the CLIVAR workshop

Hi,
Previously we have used the VM flavor hpc.16core-64ram-ssd-ephem which worked well for the FOSS4G workshop.
However, this time the amount of requested resources per user and the number of users are higher for the CLIVAR workshop. Using the same flavor hpc.16core-64ram-ssd-ephem will create a big number of VMs for the same Pangeo cluster. We should consider using the hpc.30core-64ram instead so we accommodate the same number of users with less VMs and save some overhead. See:
https://docs.google.com/spreadsheets/d/1mwDKzmlN3je5n7gVj9nq9mn4hNtbnEAKjrd0K4Vv4Vo/edit#gid=1376984822
Please let me know your thoughts.
Best regards,
Sebastian

Using larger instances will make it easier to setup the infrastructure
However, in terms of performances with Dask, is it better to use large or small flavors?
Maybe should we run some benchmarks tests, probably after the CLIVAR workshop

I believe we need to find a good balance between flavor size and the use of dask clusters.
Maybe it's too late to discuss this for CLIVAR and we need to benchmark later on.

I managed to add several VMs to pangeo-clivar which now has 16 WNs + 1 front (this is the HPC flavor with 16vCPUs and 64GB RAM)
Anything beyond that fails with this message on openstack:
No valid host was found. There are not enough hosts available.
In total pangeo-clivar has 1716=272vCPUs, and there is this other infrastructure that @guillaumeeb built which has 38=24vCPUs
So in practice we can only use 296 out of the 1736 vCPUs which are shown on the project...

Thanks, I am reporting this to CESNET and will come back to you.

While CESNET replies, could you please:

Delete VMs with Error state.
Try adding new nodes but in small batches (e.g. 5 at a time)?

@sebastian-luna-valero

I have deleted them in openstack, and that must have triggered something because they then appeared on the IM Dashboard (as "Failed") whereas before they were not even shown
I will try again, with a single VM to start with

Back to the original question, my feeling is that it's more a question of optimal ratio of memory per core in a Dask cluster (and in Jupyter notebooks) for our use cases. Having an infrastructure with ~2GiB per core seems a bit limited to me, but IIRC this is what we asked for.
I guess for workshop, it's probably OK to have only 2GiB per core, and this allows to have more people working at the same time. For real applications, this will probably vary, but again it seems low to me.

Using the same flavor hpc.16core-64ram-ssd-ephem will create a big number of VMs for the same Pangeo cluster

I don't think this is a problem. It depends on where the overhead you're talking about is.

@sebastian-luna-valero, In #21you said:

I would be very much in favor of fixing the amount of vCPUS/RAM per dask worker so we have a predicable amount of capacity, to avoid capacity problems.

As @tinaok said, we want to be able to change configurations depending on our need. However, we can also use profiles, and always keep the same core/memory ratio.
Even if we don't use profile, the idea would still be to have a "reference" ratio, that is used in general. If we change VM flavor, we should update this ratio to be 1 core each 2GiB of RAM.

CESNET confirmed that we should be using the elixir.16core-64ram flavor for new nodes in the k8s cluster.

So it's a different flavor than the one we were using? Does this implies using im-client?
Also, does this mean that the underlying physical infrastructure is kind of separated front the one used for the flavor we were using?

@sebastian-luna-valero

@tinaok @guillaumeeb @annefou @j34ni I guess at this point we have a definite number of attendees, so we can refine the total amount of capacity required?

Hi, I'm on site now, we will be 30 (22 student 8 mentor) tomorrow.
Anne will start showing https://github.com/pangeo-data/clivar-2022 8:30 tomorrow morning.

So it's a different flavor than the one we were using? Does this implies using im-client?

It is, but I am checking OpenStack and expanding the cluster via IM Dashboard picks up the correct flavor elixir.16core-64ram automatically so no need to use im-client.

Also, does this mean that the underlying physical infrastructure is kind of separated front the one used for the flavor we were using?

It means that the new worker nodes added to the k8s cluster will still use 16 vCPU cores and 64 GB RAM, but they won't use SSD disks. I am sorry but CESNET didn't have the capacity available at the moment to allocate many more VMs with the previous flavor.

Hi, I'm on site now, we will be 30 (22 student 8 mentor) tomorrow.

Thanks, please add as many elixir.16core-64ram worker nodes as possible. Once we hit the limit at CESNET we will figure out what capacity can be offered to each attendee.

Anne will start showing https://github.com/pangeo-data/clivar-2022 8:30 tomorrow morning.

I hope all goes well!

@annefou @tinaok @sebastian-luna-valero
There are currently 20 WNs and everything is green (configured), so I would rather not fiddle with the infrastructure while the course is ongoing

@tinaok
These 20 WNs currently provide:
20 * 16 = 320 vCPUs
and
20 * 64 = 1280 GB RAM
Do you want to try to add more before the Dask session?

It means that the new worker nodes added to the k8s cluster will still use 16 vCPU cores and 64 GB RAM, but they won't use SSD disks.

I think up to this point we never used SSD disk, at least for performance. For the workshops, they are clearly not needed.

User use NFS disk space, it think it is based on the SSD disk? and also when dask buffers extra memory to disk, which is also SSD.

@j34ni

@tinaok
These 20 WNs currently provide: 20 * 16 = 320 vCPUs  and 20 * 64 = 1280 GB RAM
Do you want to try to add more before the Dask session?

memory is ok.
Do we have 4 cores for each jupyterlab? Then 8 cores x 30 users so it should work for the tutorial of this morning.

It seems that sdb is of rotational type, and not SSD

/dev/sdb is where the Ceph-based volume for NFS is mounted. I wouldn't expect this to be SSDs.

I'm not sure about the Home directory. In any case this is another volume, not linked to the VM flavor. And SSD performance is not needed.

I believe home directories are using the default storage class configured by IM, which is NFS underneath

For the worker local disk, I don't think we need SSD performance for spilling during the workshops either.

That's great to know, thanks!

User use NFS disk space, it think it is based on the SSD disk? and also when dask buffers extra memory to disk, which is also SSD.

I'm not sure about the Home directory. In any case this is another volume, not linked to the VM flavor. And SSD performance is not needed. For the worker local disk, I don't think we need SSD performance for spilling during the workshops either.
@guillaumeeb @sebastian-luna-valero
I'm not sure.
Students are building their project.  So I do not know the size of data they will use during the workshop.
Depending on the size of data that they will analyse this week and next week, it might spill to RAM of dask?
Or spill is already disabled?

Users home directories are on /pv, which is on sdb

and this is the information I found on /dev/sdb:
Vendor:               QEMU
Product:              QEMU HARDDISK
Revision:             2.5+
Compliance:           SPC-3
User Capacity:        999,653,638,144 bytes [999 GB]
Logical block size:   512 bytes
LU is thin provisioned, LBPRZ=0
Serial number:        39c4eac7-0987-4fef-bf3b-a30c1daacb93
Device type:          disk
Local Time is:        Wed Oct 12 07:32:12 2022 UTC
SMART support is:     Unavailable - device lacks SMART capability.

@tinaok
I did not check before but now I can see that there are still 16 jupyters, with 26 dask-schedulers and 150 dask-workers running
How did the infrastructure withstand the load?

@j34ni @sebastian-luna-valero @guillaumeeb
Thank you very much,
We finished tutorial with dask part.  Except some user (about 3) had problem with starting dashboard when using local dask cluster, all went fine  Thank you very much,
We were rush to finish, and I didn't precise well to shutdown clusters from daskgateway, I hope it will have some timeout and cleaning.

Depending on the size of data that they will analyse this week and next week, it might spill to RAM of dask?
Or spill is already disabled?

I don't think spilling is disabled, but it can work normal spinning disks too. Anyway, spilling is almost always bad, so if a workflow began to spill to disk, I think it should be optimized.

I hope it will have some timeout and cleaning.

I'm not sure the idle timeout for Dask Gateway cluster has been set on this infrastructure. (https://github.com/pangeo-data/pangeo-eosc/blob/main/EGI.md?plain=1#L221).


Vscode extensions are put in wrong paths in master with nixpkgs-unstable

Issue description
I'm on home-manager master(a21c97d0113ebc43983ff688b2d9abddd07d6f6e) and nixpkgs-unstable (467ce5a9f45aaf96110b41eb863a56866e1c2c3c).
I enabled vscode (vscodium) with extensions, but extensions are placed in wrong position ~/.vscode-oss/extensions/share/vscode/extensions/<extension_name> instead of correct position ~/.vscode-oss/extensions/<extension_name>. And only one of extensions from the list are actually installed.
Part of configuration:
{ pkgs, ... }:
{
  programs.vscode = {
    enable = true;
    package = pkgs.vscodium;
    extensions = [
      pkgs.vscode-extensions.bbenoist.Nix
      pkgs.vscode-extensions.vscodevim.vim # Only this is installed
    ];
  };
}

Note that this behavior is almost opposite from #1260 .
I think we should swap behaviors of home-manager between 20.03 and unstable, which is, to strip the part share/vscode/extensions in nixpkgs-unstable but not do it in nixpkgs-20.03.
Meta
Maintainer CC

Technical details

ping @rycee

Sorry, I have no idea about this, I've never used vscode and don't know anything about it except that something changed in Nixpkgs unstable some time ago which broke the module and a fix was submitted in #1100. The fix accidentally got in the 20.03 branch, which I later reverted.
If this is some new breakage then I can only ask for a PR to fix it 🙁

Sorry, I have no idea about this, I've never used vscode and don't know anything about it except that something changed in Nixpkgs unstable some time ago which broke the module and a fix was submitted in #1100. The fix accidentally got in the 20.03 branch, which I later reverted.
If this is some new breakage then I can only ask for a PR to fix it slightly_frowning_face

Understood, I can look into it if I can manage it, since I use vscode.
This may relevant https://github.com/NixOS/nixpkgs/commit/0717848d7bb5adb01c7a9f293ed07676099de6f2


feat: Added Replication Time Control for Bucket Replication

Description
This change is about taking into account the recent enhancement of the AWS Terraform Provider which now allows to set S3 Replication Time Control (RTC)
Motivation and Context
Thanks to this enhancement, we will be able to provision S3 Bucket Replication Configurations with SLAs and Metrics on the Replication itself from this terraform module.
It's the reflection on the module of the closure of the following issue : https://github.com/hashicorp/terraform-provider-aws/issues/10974
Breaking Changes
There is no breaking change since I have just added the right attributes on the Replication configuration which are read from user input in the destination block of the replication_configuration attribute.
How Has This Been Tested?

[X ] I have tested and validated these changes using one or more of the provided examples/* projects

I have adapted the s3-replication example by adding the right input and I was able to successfully see metrics on the AWS S3 Bucket Console.
There was no need to add any control over user input since AWS API provides meaningful error messages in case user input is wrong :
│ Error: expected replication_configuration.0.rules.0.destination.0.replication_time.0.minutes to be in the range (15 - 15), got 16
│
│   with module.s3_bucket.aws_s3_bucket.this[0],
│   on ../../main.tf line 5, in resource "aws_s3_bucket" "this":
│    5: resource "aws_s3_bucket" "this" {
│
│ Error: expected replication_configuration.0.rules.0.destination.0.replication_time.0.status to be one of [Enabled Disabled], got yes
│
│   with module.s3_bucket.aws_s3_bucket.this[0],
│   on ../../main.tf line 5, in resource "aws_s3_bucket" "this":
│    5: resource "aws_s3_bucket" "this" {
│
│ Error: expected replication_configuration.0.rules.0.destination.0.metrics.0.minutes to be in the range (10 - 15), got 16
│
│   with module.s3_bucket.aws_s3_bucket.this[0],
│   on ../../main.tf line 5, in resource "aws_s3_bucket" "this":
│    5: resource "aws_s3_bucket" "this" {
│
│ Error: expected replication_configuration.0.rules.0.destination.0.metrics.0.status to be one of [Enabled Disabled], got true
│
│   with module.s3_bucket.aws_s3_bucket.this[0],
│   on ../../main.tf line 5, in resource "aws_s3_bucket" "this":
│    5: resource "aws_s3_bucket" "this" {
│
In this case the module users, will be easily able to correct their input to feed with the right values.
I have also ran pre-commit hooks as per the original pre-commit config file.
Thanks again for your review and feedback.
Bests.

Please use terraform-docs version 0.13.0 and update the minimum version of AWS provider to be 3.64.0.

Done in the last push.
Thanks again.
Bests.

Thanks, @schniber !
v2.11.0 has been just released.

Thanks a lot @antonbabenko !!!


Different signature when signing with bitcoinjs-message and with Electrum

I am trying to sign a message with a nested segwit address (3*). I got the address, the message and the private key, and also opened the wallet on Electrum. However, when signing with Electrum I get a different signature compared to when signing with bitcoinjs-message with the following code:
var signature = bitcoinMessage.sign(
    'test',
    privateKey,
    keyPair.compressed,
    {segwitType: 'p2sh(p2wpkh)'},
);

Do you have any idea what the issue could be? Thanks for your help and for the great work!

There is no guarantee that signatures are deterministic. Or that they are deterministic in the same way.

Sign with Electrum and verify with us.
Sign with us and verify with Electrum.

Please perform the two actions above and tell me if anything fails.
If both succeed this issue should be closed. Thank you.

Thank you very much Jonathan for your quick reply.

Signing 'test' with Electrum and verifying with bicoinjs-message returns false
Signing 'test' with bitcoinjs-message and verifying with Electrum returns false (Wrong signature)

I realised that I have the same issue as in #20 where only the flag byte is different for simple strings (like 'test'). But when using a more complex message to sign the signatures are completely different.

I just would like to mention that I didn't have this problem with legacy addresses. I am using the same code as I did previously with a legacy address, here the only difference is the p2sh(p2wpkh) address, hence the {segwitType: 'p2sh(p2wpkh)'} option (and the private key obviously)

Sounds like Electrum differs from Trezor then.
non-legacy message signatures are not standardized, and we just followed Trezor.

ah, wait.
Are you converting the private key to the Electrum format for p2sh-p2wpkh?
That might be it

No, I am not converting the key, using directly the private key and then using:
const keyPair = bitcoin.ECPair.fromWIF(pkey)

I am also wondering if the string handling is the issue, as Electrum probably considers \n differently than JS. I will come back to you after some more tests and close the issue if I found a solution.

https://github.com/bitcoinjs/bitcoinjs-message/pull/29#issuecomment-686060820

Thank you @junderw, solved with the PR mentioned above!


Revisit the HFC Query logic to shift DiskLedgerViews

The logic is somewhat brittle and even if the types align I'm somewhat afraid it might not be doing exactly what we want. This requires some careful auditing. In particular:

The indices when shifting a ledger state might be off
The logic for distributing the queries might be brittle. Are we sure we can distrib them always to the current era and the queries will work?

This was Mob-reviews so let's close it


Google Map maxZoom seems to not work

Hello,
I have problem with the mapOptions of my map.
I would like to restrain the map zoom to 15 because I am doing a Geospatial Queries.
My code looks like this
Template.mypage.helpers({
  mapOptions: function(){
    if(GoogleMaps.loaded()){
      return {
        center: new google.maps.LatLng(48.8520442,2.3346246),
        zoom: 13,
        maxZoom: 15
      }
    }
  }
})

Unfortunately the maxZoom seems to not work...
Are they any other possibility to restrain the zoom ?

Alright I posted a solution on stackoverflow
The solution was pretty simple, maxZoom and minZoom have to be declare in order to work !


Add a WHERE clause for attribute

Summary
When checking upon an attribute, you might need to filter using a WHERE clause. For example if you're sending alerts for multiple sub-accounts to a single account, you need a way to filter on which account you are displaying in a row/column
Desired Behaviour
Filter the result of an attribute based on some value.
Possible Solution
When selecting a row/column/attribute/operator and value, there could be a tickbox to define a WHERE clause where you pick an attribute, operator (IN, NOT IN, LIKE) and a value.
Additional context
Sending alerts from multiple new relic sub accounts to a single account.

We do this very thing and having a where or facet would be really useful is showing which account the issue is in, the facet function would allow us to be more dynamic but a where clause would work.

I've forked the repo and am currently working on an enhancement to allow this.

@danifitz excited to see that you were working on this feature. If you have a PR, @glitton or @amit-y would be happy to review and merge it so that functionality can find it's way into the New Relic One Catalog.


Front: add project name, package and port to front generator

rename bounded context generator to springboot
add generator object to API and update form
"generator-jhipster": {
"baseName": "beer",
"projectName": "Beer Project",
"packageName": "tech.jhipster.beer",
"serverPort": 8080
}

$100 bounty claimed => https://opencollective.com/generator-jhipster/expenses/74248

@Franceq34 : approved


Update Composer dependencies (2021-05-04-00-03)

Visual regression test passed!
View the visual regression test report


Skip failing tests

Skip these tests in Preview2. Last one was accidentally against dev.

@ryanbrandenburg,
Thanks for having already signed the Contribution License Agreement. Your agreement was validated by .NET Foundation. We will now review your pull request.
Thanks,
.NET Foundation Pull Request Bot


Update the nexus module

This PR addresses issue#93 and updates data to the nexus module, addresses the tests issue

How to review
TBD

PR checklist
TBD

[x] Continuous integration checks all ✅

[x] Add or expand tests; coverage checks both ✅

[x] Add, expand, or update documentation.

[x] Update doc/whatsnew.

Currently, the linting test is failing (among others). Please see our guide for contributing code to see what code style we expect. Please also note that black, flake8, mypy, and isort can all be configured to be applied automatically whenever you save your code (the details of which can vary, but we can work it out together if you want to). This helps passing theses tests tremendously.

The current issue with the other tests seems to be that they still rely on code in message_data (which they shouldn't, as explained before, since all code here should be open source):

message_ix_models/tests/model/water/test_nexus.py:5: in
from message_data.model.water import build
E   ModuleNotFoundError: No module named 'message_data'

Also to make sure: how large are these data files that you are updating here? If they are too large (and there are lots of them), it might be better to upload them to Zenodo or so (see the second point in the comment by @khaeru).

Changed to "draft" status; prefer this over putting "[WIP] " in the title, as it means the same thing.
I would recommend to update the branch by "Update with rebase" and then git pull --rebase on your local.

The tests are mostly working, despite what it looks like here. There are some general issues however that we need to address to fully see their potential:

The most generic problems arise from the Code quality check. This tells us that the code is not formatted correctly and that there are some issues like unsorted import blocks and unused variables. As mentioned before, tools like ruff (superseding isort and flake8) and black can be set up to automatically run whenever you save a file, eliminating most of these problems.
Some of the Code quality problems are not as easy to resolve. Things like unused variables need manual attention, even though they are simple: if you truly don't need the return value of a function, you can use an underscore: _ = some_function(). The mypy errors can be more tedious to resolve as we need to understand why the mentioned modules don't have the missing attributes and whether they should have or where else they should get them from.
As mentioned before, the tests only contain a limited number of problems:

ERROR message_ix_models/tests/model/water/test_cooling.py
ERROR message_ix_models/tests/model/water/test_utils.py
FAILED message_ix_models/tests/model/water/test_irrigation.py::test_add_irr_structure - AttributeError: 'dict' object has no attribute 'regions'
FAILED message_ix_models/tests/model/water/test_water_supply.py::test_map_basin_region_wat - AttributeError: 'dict' object has no attribute 'time'
FAILED message_ix_models/tests/model/water/test_water_supply.py::test_add_water_supply - ModuleNotFoundError: No module named 'your_module'
FAILED message_ix_models/tests/model/water/test_water_supply.py::test_add_e_flow - ModuleNotFoundError: No module named 'your_module'

For the first two FAILED, the issue is the same: the actual function is using the context object incorrectly:

    info = context["water build info"]

    if "year" in context.time:

Are the lines from the actual code. I don't know if this should work, but it's treating context in two different ways: the first reminds me of a dictionary, the second of class. In your tests, you set up a mock Context object (which would normally be a class) as a dictionary, which is why the info = ... line works, but the second one complains: you cannot access dictionary data that way. Since I doubt you want to rewrite your actual code function to use both dict and Context for the context argument, I suggest to study how other tests in this repo solve this problem. You should be able to use a test_context out of the box which you can then adapt.

The last two FAILED result from trying to call a module called your_module, which doesn't exist. You probably want to replace your_module with message_ix_model.model.water.data in both cases.
Finally, both ERRORs are due to missing imports. Both of these issues can probably be solved by addressing all mypy complaints, although one of them is trying to import message_data. This should not happen in message-ix-models and might be a sign that the clean-up process is still incomplete.

@awais307 can you please update on what your commits did,  if anything is still missing and how you expect others to review? thanks

@awais307, @adrivinca: the tests are almost working, you're on a good path there! Almost all errors seem to originate from the same problem: that you are not using a 'real' Context object. As pointed out above, using those is preferable since it makes the tests easier to understand and maintain and since you won't have to rewrite your actual code to deal with both Contexts and dicts.
With the latest commit, I adapted the test in tests/model/water/test_cooling.py to show you what I mean by that. We do have an object called test_context and we can use it just like that our test functions. Please take a look and try to adapt your other tests in a similar way, I suspect it will solve most of the remaining errors :)
If you don't want to write the same setup of the text_context all the time, you could introduce another fixture like it e.g. in the tests/model/water/__init__.py: you can define a function that takes the test_context, does whatever setup all your tests frequently share, and return the modified test_context. You can then use the pytest decorator @pytest.fixture(scope="function") to make this function work identical to test_context. Then, you can use it in all your other tests as well :)
One test error might remain, though, and that is this one:
___________________________________________ test_map_yv_ya_lt ___________________________________________

    def test_map_yv_ya_lt():
        periods = (2010, 2020, 2030, 2040)
        lt = 20
        ya = 2020

        expected = pd.DataFrame(
            {"year_vtg": [2010, 2020, 2020, 2030], "year_act": [2020, 2020, 2030, 2040]}
        )

        result = map_yv_ya_lt(periods, lt, ya)

>       pd.testing.assert_frame_equal(result, expected)
E       AssertionError: DataFrame are different
E
E       DataFrame shape mismatch
E       [left]:  (8, 2)
E       [right]: (4, 2)

message_ix_models/tests/model/water/test_utils.py:124: AssertionError

The shape of the compared DataFrames won't be fixed by the different context, I assume. Please check that expected is really what we expect, e.g. year_vtg contains 2020 twice, is that on purpose?
One final note: I removed a few markers that told rust to ignore functions being too complex. Please take another look at those and try to reduce their complexity! In most cases, it is enough to take a part of the function that includes a lot of if ... elif ... else ... and move that to a new function that takes some input data and returns different output depending on the condition.
Please let me know if you need help with any of the above :)

Hi @adrivinca and @awais307, I've taken another look at this PR. I started by addressing all code quality issues: applying some formatting and reducing code complexity. The latter worked quite well in parts because you already had all the puzzle pieces present: e.g. in water_for_ppl.py, your immensely complex cool_tech() function was in part only complex because you defined various helper functions inside that same function. So by moving the definitions outside, the code quickly became less complex.
Unfortunately, the same approach did not work as well for infrastructure.py. While moving stuff outside the main function also resolved the issue, there's still much room for improvements. Almost all steps in the main function that work with the input DataFrame seem to be very similar except for tiny variations, so according to the DRY principle (Don't Repeat Yourself), this code is most likely generalisable.
Next, I took a look at the failing tests. For some of them, I was able to fix them myself, though even in those cases, there are some open questions. Please check e.g. the TODOs in test_map_yv_ya_lt().
For some cases, trying to use the test_context or otherwise fixing the tests led to problems to such a degree that I didn't know how to fix them. Please search for FIXME to find descriptions of these errors with probable causes and determine how you want to fix them.
Finally, your reporting.py contains these lines: https://github.com/iiasa/message-ix-models/blob/abcd6144752a742e9191cd4d51be3bd47de020e4/message_ix_models/model/water/reporting.py#L14-L36
run_old_reporting() is part of your main workflow (via report_full()), so legacy_reporting is actually required to be there. My version of mypy is already marking this as an error because legacy_reporting really cannot be None, so your code still requires message_data to be present, although no code in this repo should. Please find a solution for that, e.g. by not using the legacy_reporting at all and instead using the reporting system that message_ix_models provides. At the very least, you should add a clause/flag to report_full so that run_old_reporting() is only run if message_data is present. For this, you could reuse the HAS_MESSAGE_DATA flag that already exists in the code: https://github.com/iiasa/message-ix-models/blob/abcd6144752a742e9191cd4d51be3bd47de020e4/message_ix_models/util/common.py#L21

Thanks a lot @glatterf42 for spending time on this and for the thorough overview.
I am going through the tests and the code, here the main changes I did

Taking from another commit I did in the leap-re-wks branch, I replaced all the  .append() with pd.concat([])  and also another couple of parts where the code was obsolete
concerning test_map_yv the test and functions work as intended. I just don't know why, when running locally the reset_index() in the main function does not seem to work when called from another module.
I used if HAS_MESSAGE_DATA: in the reporting.py script to use the legacy reporting. Not sure the the reporting features in message_ix_models are outputting the same as legacy

Some FIXME are related to making a more general configuration for the module, and also maybe a testing config.
I would like to work on it for the SSP update, where I need anyway to change the code and for instance add the SSP config.
Therefore, I left some FIXME for the future, and I would merge them with the FIXME memo for now
I refer to the comments in test_irrigation, test_water_supply.
Other points are also open questions to me and I would like @awais307 to please intervene and fix.
in test_water_supply:

test_add_e_flow function does not make sense to me, how is the function supposed to work with fictitious regions if the files that get loaded are data files?

currently, the function is checking to match this df
   Region  value  year  time  Unnamed: 0  BCU_name
0  test_Region      1  2020  year           0  test_BCU

and df_sw in the function read_water_availability() before it breaks
     Region     years        value
0  test_BCU    Region  test_Region
1  test_BCU     value            1
2  test_BCU      year         2020
3  test_BCU      time         year
4  test_BCU  BCU_name     test_BCU

I think these functions need to be tested with actual existing data

similar for test_add_water_supply(), please look at it, because it breaks, ut not sure if it is supposed to work with the input data that you give
same for test_map_basin_region_wat()
unfortunately I am not familiar with the with   patch and mocking approach @awais307  have you tried those tests you wrote?
When running pytest in the folder the al break
please also have a look to the annotation issue in test_utils.test_add_commodity_and_level(). and test_utils.test_read_config

@awais3

@awais307, I'm trying to keep the commit history linear by using git rebase main. A linear history means it will be very easy to understand how and why which changes were introduced and the changes will integrate with the rest of the code base neatly. So if you see that this branch is out-of-date with the main branch, I'd appreciate if you could also use git rebase instead of git merge :)
If you don't feel comfortable using git rebase, please check out e.g. this tutorials.

@awais307, I'm trying to keep the commit history linear by using git rebase main. A linear history means it will be very easy to understand how and why which changes were introduced and the changes will integrate with the rest of the code base neatly. So if you see that this branch is out-of-date with the main branch, I'd appreciate if you could also use git rebase instead of git merge :) If you don't feel comfortable using git rebase, please check out e.g. this tutorial.

okay, for some reason, my local tries to merge it whenever I fetch the changes.

@awais307, I'm trying to keep the commit history linear by using git rebase main. A linear history means it will be very easy to understand how and why which changes were introduced and the changes will integrate with the rest of the code base neatly. So if you see that this branch is out-of-date with the main branch, I'd appreciate if you could also use git rebase instead of git merge :) If you don't feel comfortable using git rebase, please check out e.g. this tutorial.

okay, for some reason, my local tries to merge it whenever I fetch the changes.

Could you please try to configure your local git interaction not to do that, then? If you tell me how you interact with git locally, I might be able to help you. Do you use Github Desktop? GitKraken? The command line?
To get the changes from up here to your system and overwrite the changes you have locally, you might need to perform something like a "force pull", which you can do following these instructions. Please let me know if you need any help with that :)

Hi @awais307 :)
I took a closer look at the failing tests today and most of them should be fixed. Currently, I just see one local failure, which comes from the test_build.py::test_build which you recently added. Do you have any kind of confirmation that the water module functions should work with the bare_res scenario?
The test currently fails in the function cool_tech, which is called as part of model/water/data/__init__.py::add_data(), which in turn is part of apply_spec() as called in model/water/build.py::main(). The code complains that you can replace df[[label1, label2]] with something applied to df[[label1, label2, label3]]. If I adapt the lists of labels to fit, however, I run into another error a few lines later which states that we are trying to fill None values in cooling_df with values in ref_input["technology"], but these values are all None themselves. So I suspect something is going wrong at a deeper level.
I also suspect that adding a test for the function cool_tech() will shine a light onto why this other test is failing.
In any case, I hope the examples for how to use the test_context that are now included in the test suite help you with figuring out how to use it for testing further cases :)

I've added several new (superficial) tests that should increase the test coverage enough to make this PR mergeable :)
However, there are two main notes:

By superficial I mean that the tests don't check any actual resulting values, they just check that the results have roughly the correct form (e.g. column names are matching). This could be improved to actually make the tests reliable.
Some of the new tests are not passing. If marked them with FIXME and added notes about what I think is going wrong. Often, it's a question of "we want to use a Scenario to e.g. run the nexus reporting on, but for this we first have to create a Scenario properly, so that e.g. all cooling technologies can be found in the input parameter". I expect that you are much more familiar with how to do that, so I'd leave that up to you, @awais307 and @adrivinca.

I've added several new (superficial) tests that should increase the test coverage enough to make this PR mergeable :) However, there are two main notes:

By superficial I mean that the tests don't check any actual resulting values, they just check that the results have roughly the correct form (e.g. column names are matching). This could be improved to actually make the tests reliable.
Some of the new tests are not passing. If marked them with FIXME and added notes about what I think is going wrong. Often, it's a question of "we want to use a Scenario to e.g. run the nexus reporting on, but for this we first have to create a Scenario properly, so that e.g. all cooling technologies can be found in the input parameter". I expect that you are much more familiar with how to do that, so I'd leave that up to you, @awais307 and @adrivinca.

Thanks a lot @glatterf42 . Very much appreciated for the contribution and support. Per the tests in demands, I think perhaps @adrivinca can confirm better since he did most of the recent adjustment with Zambia model. In principle, I would assume there is no need to read target rate function for a country case since those are meant for global basins.

one of the problems that Fridolin mentioned has to do with this function
def get_basin_sizes(basin: pd.DataFrame, node: str) -> Tuple[float, float]:
    """Returns the sizes of developing and developed basins for a given node"""
    temp = basin[basin["BCU_name"] == node]
    print(temp)
    sizes = temp.pivot_table(index=["STATUS"], aggfunc="size")
    print(sizes)
    return sizes["DEV"], sizes["IND"]

I solved it by defining the sizes =0 in case they are empty before outputting.
Also the test function needed a different "SDG" in the case of the ZMB model, which I changed.
The other issue on test_wateR_for_ppl::test_cool_tec shows multiple issues that I adjusted:

the test requires some parameters (input, historical_activity and historical_new_capacity ) to exist in the starting scenario, so I added them in the text
the input RCP need to be "no_climate" instead of "no-climate" because of the input data files
one part that Fridolin changed needed to be reverted
one update in cooling_fr() related to pandas updates that I did in other branches was not included here. So I fixed it

But i see some of the solutions that work for Windows do not work on Linux, for instance,
message_ix_models/tests/model/water/data/test_water_for_ppl.py::test_cool_tec[no_climate] - KeyError: 'parent_tech'
it should read a column from the incoming df, not sure why it gives an error. Any ideas @glatterf42
Thanks!

Hi @adrivinca, thanks for taking another look at this :)
I've tried addressing some of the issues here. As you noticed, there were some test failures that only occurred for tests that use ixmp and message_ix v3.6.0 (and older), so I've marked the function as needing a minimum version of 3.7.0. For 3.6.0 and older, our test suite forces pandas < 2.0.0, but one function in your tests uses a parameter introduced with pandas 2.1.0. This is totally fine, and if it happens again (or I forgot a function calling add_sectoral_demand), please apply the same changes that are now in https://github.com/iiasa/message-ix-models/pull/106/commits/feafa7edfd47d83eac78a15ba98263ef42686ef8.
Next, I noticed that you removed the updated assignment in model/data/water_for_ppl.py:256 that I previously changed. Without my changes, I see a pandas error because
    ref_input[["value", "level"]] = ref_input[["technology", "value", "level"]].apply(
        missing_tech, axis=1
    )

tried to assign three colums of a dataframe to two columns (of the same dataframe, which makes me think there should be a better way to simply update these two columns in the first place). This does not work because the number of columns on both sides of the assignment needs to match. So, assuming the dataframe you want to apply your missing_tech function to requires the technology column and you don't want to assign this column in ref_input again, I've now changed it to this instead:
    ref_input[["value", "level"]] = ref_input[["technology", "value", "level"]].apply(
        missing_tech, axis=1
    )[["value", "level"]]

Selecting the columns we want to assign after they have been re-computed. For this to work, I had to also adapt missing_tech to return a pandas Series with correctly named columns.
With this, test_build still runs into an error: NaN values can't be converted to int. This seems to be the result of almost all dataframes in cool_tech being empty in this test, in particular ref_input in line 264. I'm not sure why this is the case and if this is supposed to work. So please clarify if cool_tech should be able to handle this case of empty dataframes, or if this is by mistake and these dataframes should never be empty. In the latter case, you could consider introducing a quick sanity check, either with try...except... to raise an error message or a warning if the dataframe is empty or with assert ... to ensure the dataframe is not empty (and raise an AssertionError otherwise).
Finally, we still have the error in test_report_full, which arises from the call to legacy_report. From my adaptation of the legacy reporting to message-ix-models, I remember that this error most likely comes from the fact that the default reporting is trying to calculate tables (the default tables, presumably), that you don't actually want to compute. Please take a look at your local reporting configuration and ensure this branch is computing the same tables. If this doesn't help, @OFR-IIASA is the expert on message_data-style reporting and can probably quickly point you to what's wrong if you ask nicely :pray:

For completeness' sake: there is an error about a dimensionless unit. This is completely unrelated to this PR and will be fixed upstream in pint at some point. Please disregard for this PR :)

Thanks @glatterf42 , sorry for the long delay, but it's hard to find time for this.
Concerning the code you changed, I understood your changes from before. but on my machine it behaved differently, like as the function needed 2 inputs and had 3 outputs... maybe some differences in packages?
concerning the reporting, I did not change it to the legacy version, which I never used, so I was expecting that whoever updates the code also make sure it works...
Still, I will dedicate time to solving those two issues over the next two days. But if some errors remain too complicated I would ask to please proceed with merging asap.
There are updates to be done that might drastically change the module, and the tests accordingly. So I would rather keep some tests commented and get back to them after the updates.
Cheers

No worries, thanks for dedicating some time now :)
For the packages, the error might indeed come from different versions. Trying to work with this PR from a new virtual environment (as you already indicated you would on Slack) sounds like the best way to solve the issue :)
Please help me understand what is going on with the reporting, then:
https://github.com/iiasa/message-ix-models/blob/cf687da50760239cf72d488a662fbc08bd583907/message_ix_models/model/water/reporting.py#L20C1-L32C6
This function is called run_old_reporting() and, looking at the blame, was called so and written almost entirely by you. It mainly calls the reporting system from message_data, which is called "legacy_reporting" here.
Back in the original migration PR, you stated that your code relies on the report() function from message_data: https://github.com/iiasa/message-ix-models/pull/88#issuecomment-1436986943
@khaeru then suggested a workaround to limit importing this function to only those systems that have message_data installed.
This is not really a good solution, though, since our open-source modules should not rely on closed-source modules. Thus, #159 took the version of report() from message_data that was on ssp_dev at the time and plugged it into message-ix-models. We don't make any guarantees that this works with any code except for one: this reporting function works with the global baseline snapshot we published on Zenodo.
Still, with https://github.com/iiasa/message-ix-models/pull/106/commits/1ecad356ebb9eca47b9e062c36b7ad96d136b337, I migrated the water-report-legacy_report() call to the new message-ix-models-legacy_report(). If this doesn't work out of the box, you essentially have a few options:

Migrate back to the conditional import from message_data
Adapt the legacy_report functionality here so that it satisfies both our working test and your new one
Migrate your code so that it only uses the new reporting feature

Arguably, the first option is the least work and the worst style, while the last option is the most work, best style, and highly appreciated by everyone else who would also benefit from this.
Please let me know if I got that right and what you intend to do :)
Finally, please note that I force-pushed this branch to integrate the latest changes from main. So please git fetch this branch if you have local changes and git rebase your local changes on top of this version here. My code improvement tools signal that some things are not working correctly immediately, so I will also draft a small commit or two addressing these issues.

Looking at the test failures here:

ERROR message_ix_models/tests/tools/test_wb.py::test_get_income_group_codelist - requests.exceptions.HTTPError: 502 Server Error: Bad Gateway for url: http://api.worldbank.org/v2/sdmx/rest/codelist/WB/
ERROR message_ix_models/tests/tools/test_wb.py::test_assign_income_groups[R12-population-None] - requests.exceptions.HTTPError: 502 Server Error: Bad Gateway for url: http://api.worldbank.org/v2/sdmx/rest/codelist/WB/
ERROR message_ix_models/tests/tools/test_wb.py::test_assign_income_groups[R12-count-None] - requests.exceptions.HTTPError: 502 Server Error: Bad Gateway for url: http://api.worldbank.org/v2/sdmx/rest/codelist/WB/
ERROR message_ix_models/tests/tools/test_wb.py::test_assign_income_groups[R12-population-0] - requests.exceptions.HTTPError: 502 Server Error: Bad Gateway for url: http://api.worldbank.org/v2/sdmx/rest/codelist/WB/
FAILED message_ix_models/tests/tools/test_wb.py::test_make_map - requests.exceptions.HTTPError: 502 Server Error: Bad Gateway for url: http://api.worldbank.org/v2/sdmx/rest/codelist/WB/

See the commit on #213 with the subject "Use HTTPS to access World Bank SDMX endpoints / Work around khaeru/sdmx#191". You could cherry-pick this commit onto the current branch.

FAILED message_ix_models/tests/model/transport/test_report.py::test_plot_simulated[stock-ldv] - genno.core.exceptions.MissingKeyError: required keys ('fuel economy:nl-m:ldv+ref',) not defined
FAILED message_ix_models/tests/model/transport/test_report.py::test_iamc_simulated - genno.core.exceptions.MissingKeyError: required keys ('fuel economy:nl-m:ldv+ref',) not defined

These I am also seeing in #213 and struggling to reproduce locally, so I do not yet have a fix to merge or for you to cherry-pick. You could mark these with:
@pytest.mark.xfail(reason="Temporary, for #106")

…and make a note in the PR description that you have done this.

Let me look briefly at the other two…

E           pandas.errors.IntCastingNaNError: Cannot convert non-finite values (NA or inf) to integer: Error while type casting for column 'year_act'

This indicates that all-NaN values are present in a "year_act" column in one of the variables used in this statement: https://github.com/iiasa/message-ix-models/blob/73ab3e4e3343ce7d6b425ed0ce4a9402c9eb1012/message_ix_models/model/water/data/water_for_ppl.py#L261-L266
You could debug by displaying these variables just before this statement and identifying why this happens.

FAILED message_ix_models/tests/model/water/test_report.py::test_report_full - SystemExit: 1

As far as I can see: https://github.com/iiasa/message-ix-models/blob/73ab3e4e3343ce7d6b425ed0ce4a9402c9eb1012/message_ix_models/tests/model/water/test_report.py#L12-L46
…this test sets up a new, completely empty scenario; builds MESSAGEix-Nexus on top of that; solves it; and then runs the report_full() function, which in turn invokes the legacy reporting.
Looking at the exception raised here in .report.legacy.postprocess._retr_land_act(), we can conclude a few things:

The legacy reporting code expects some values in the LAND variable, and cannot (or just refuses to) proceed without them.
This is not documented.
The code calls sys.exit(1) directly instead of the Pythonic behaviour of raising an exception.

There are various ways (not mutually exclusive) to approach this:

Expand the test such that LAND is non-empty in the solution. This could involve:
a. Create minimal ("dummy") parameter values that achieve this.
b. Build on the snapshot scenario instead.
In .water.report, adjust:
https://github.com/iiasa/message-ix-models/blob/73ab3e4e3343ce7d6b425ed0ce4a9402c9eb1012/message_ix_models/model/water/report.py#L26-L31
to:try:
   legacy_report(...)
except SystemExit:
   log.warning("Legacy reporting failed")

(However please note that message_ix_models.report.report(...) is the preferred way to invoke the legacy reporting. That could also be the proper place for this protective code. If you decide to use this approach, please help me in and I can jump in.)
Improve the documentation of the conditions where .report.legacy is usable.

Thanks Paul and Frido.
I am looking into the build issue.
Concerning reporting, I think the legacy reporting does not work if a scenario is empty, The scenario requires the globiom matrix emulator (that's why we have LAND related error).
So if you want to make sure it is somehow testable, maybe who migrated the legacy reporting could do a test that tests a normal ENGAGE (or SSP) scenario first in another PR. Then that could be taken as an example for other branches. Otherwise I am afraid we are charging too much stuff on this PR
I also tried to only run the water reporting and I think it also work only with actual scenarios. Some operation will not work if there is not output (I already get an error from "Reporter")
So, if possible I would not test it right now - eventually just test the imports of the reporting file- and leave it for the next update to prepare a nexus_bare small scenario for testing. But I don't want to do it now because there are major changes coming.
Does it make sense?

Thanks for your analysis @khaeru, I have cherry-picked your commit as suggested.
I have also tried to address the numerous mypy issues, of which I don't understand how they were not caught with our code quality check here before. Sometimes, these approaches include 'best guesses' as to what the code should do and I tried to mimic that, but I might have been wrong, so @adrivinca please check that the code still works as intended.
I was the one who migrated the legacy reporting. As far as I know, the snapshot on Zenodo is a baseline snapshot for ENGAGE. Other than that, I don't know what a "normal scenario" is supposed to be and where I could get one; I certainly don't have the time to build one from scratch.
Agreed, making drastic changes to legacy_report() is out of scope for this PR. However, if I remember correctly, legacy_report() tried to run different "reporting tables" including those for LAND output. These could be disabled pretty easily, but I don't remember how exactly we did that. It might be documented in https://github.com/iiasa/message-ix-models/pull/159, though.
In general, as @khaeru suggested, you are free to mark certain tests as XFail or so, but please record in a new issue which tests you marked and why and try to address them soon (if after your upcoming major changes).

So, if possible I would not test it right now - eventually just test the imports of the reporting file- and leave it for the next update to prepare a nexus_bare small scenario for testing. But I don't want to do it now because there are major changes coming.
Does it make sense?

Yes, this makes sense and is a fair strategy. As @glatterf42 says, you can still retain the test_report_full() that is added on this branch, as a starting-point for that future work. Just mark it XFAIL so that pytest understands that is expected:
@pytest.mark.xfail(reason="Incomplete")
def test_report_full(...):
    ...

We should now be at the same two errors we had before the mypy commit, I'll leave it at that for now.

Apologies, I was one week on leave and I left it pending. Thanks again for your effort.
I was looking at the test_build.py issue and simply the script cannot work because the scenario given as input is basically empty, for instance, it does not have an input parameter. the script runs over all the other scrips (like water_for_ppl, demands.py and so on) and, like the test that were testing those functions, it would need some parameters to start with.
for instance in water_for_ppl line 242, ref_input is empty n the test scenario
    ref_input: pd.DataFrame = scen.par(
        "input", {"technology": cooling_df["parent_tech"]}
    )

as a consequence input_cool is empty and the code gives error, line 262
input_cool = (
        cooling_df.set_index("parent_tech")
        .combine_first(ref_input.set_index("technology"))
        .reset_index()
    )

I could try to add manually some dummy parameters as did in test_water_for_ppl.py and see if it works. But for this I have a question. I see the starting scenario in test_build is defines as scenario = testing.bare_res(request, test_context), while in test_demands and test_water_for_ppl it is defined as s = Scenario(**scenario_info). What is the difference and what is recommendable?
Anyway I am not sure the build would work with a semi-empty scenario. sometimes, specific data is needed. Also, I would assume that if the functions that build calls are already tested I would check something

Hi, as motivated in the previous messages, I marked two tests with @pytest.mark.xfail(reason="Temporary, for #106"), namely
test_report_full in test_report.py
and test_build in test_build.py
as they both require a fully parametrized scenario as a starting point, which is not the case for the "bare" scenario for tests.
Since we are about to do major revision and update to the module, we decided to postpone investing further time in those tests.
all tests seem to pass for py3.12, not for py3.11, is that a problem?
otherwise I would ask to kindly merge.
thank you very much for the great support!

All tests seem to be passing now :)
So in light of the above discussion and with the explicit promise to write tests for the water module as soon as possible after your upcoming major revision, I think we can merge this :)


doc(load balancer): Expand detail on manual LB configuration

My only thing here is that we should probably remove all of the double spacing after the periods to be consistent with other documentation.

@jchauncey, @arschles had similar feedback about some of the double spaces after a period.  Technically, that is correct, however, I do realize that it's a common practice in tech documentation to use only one.  That being said, I'm certain that our docs are absolutely littered with instanced of single spaces and instances of double spaces following periods.  At the end of the day, both are displayed the same in a browser.  I'd be happy to take them out if that's what it takes to get this PR through, but taking them out won't make us consistent since we're already consistently inconsistent on this matter.

@krancour you're right they are removed in the browser i forgot about that.


Good to Know

Type: Feature Request
I wrote asking for help getting vscode to run in ARM while running Python in Rosetta on an M1 Mac.  Didn't get anywhere. Did find a solution which you might want to post somewhere that works.  The last line is important otherwise it doesn't work.    Hope someone else can benefit from it.  You're welcome.
"terminal.integrated.profiles.osx": {
    "rosetta": {
    "path": "arch",
    "args": ["-x86_64", "zsh", "-l"],
    "overrideName": true
    }
},
"terminal.integrated.defaultProfile.osx": "rosetta",

VS Code version: Code 1.85.1 (Universal) (0ee08df0cf4527e40edc9aa28f4b5bd38bbff2b2, 2023-12-13T09:48:06.308Z)
OS version: Darwin arm64 23.2.0
Modes:

Thanks for posting this
Please consider posting this to stack overflow too as a self answered question as those tend to show up better in search results


[CELEBORN-494][PERF] RssInputStream fetch side support blacklist to avoid client side timeout in same worker multiple times during fetch

What changes were proposed in this pull request?
####Test case
executor instance 20

SQL:
SELECT count(1) from (select /*+ REPARTITION(100) */ * from spark_auxiliary.t50g) tmp;

create connection timeout 10s

Fetch chunk timeout 30s

In the graph, the shuffle read time of before and after is always the same delay time.
Worker can't connect
Before

After

OpenStream stuck
Before

After

Fetch chunk stuck
Before

After

Why are the changes needed?
Does this PR introduce any user-facing change?
How was this patch tested?

ping @waitinfuture @RexXiong @FMX

Thanks for the PR @AngersZhuuuu ! Test cases seems very positive in worker-unavailable scenarios. I'm thinking whether we should add mechanism to remove workers from blacklist, since we can't guarantee the worker is broken or caused by fluctuation.

Test cases seems very positive in worker-unavailable scenarios. I'm thinking whether we should add mechanism to remove workers from blacklist, since we can't guarantee the worker is broken or caused by fluctuation.

For the same PartitionLocation, if the peer has failed, how about allowing the blacklisted machine to retry?

Shall we add a timeout for blacklisted worker?

Test cases seems very positive in worker-unavailable scenarios. I'm thinking whether we should add mechanism to remove workers from blacklist, since we can't guarantee the worker is broken or caused by fluctuation.

For the same PartitionLocation, if the peer has failed, how about allowing the blacklisted machine to retry?

In this PR, if both workers are blacklist, then the fetch will fast fail, maybe it's not what we want. We can add timeout for each blacklisted worker, and only consider a worker unavailable if at least two(configurable) exceptions happens.

@waitinfuture How about current?

Hi @AngersZhuuuu , this PR seems very necessary, it'll be very nice if you make it done : )

It seems like we are mixing up the terms "excludedWorker" and "blacklist".
First of all, I don't object "blacklist", but we should unify them.
There are some options I think:

use "blacklist" anywhere
use "exclusion" anywhere, even it does not mention excluding what, but it should be clear we are excluding bad workers
use "excludedWorker" or "workerExclusion" based on the context.

+1, shall we do code refactor before further related PRs?

After this pr I will make a new pr to unify the blacklist and execluded related configuration name.
Any more suggestion? @pan3793 @waitinfuture

Any more suggestion? @pan3793 @RexXiong @waitinfuture

LGTM, thanks! I'm thinking whether we should unify push blacklist and fetch blacklist. cc @AngersZhuuuu @pan3793 @RexXiong


add edit_merge_request_comment

add edit_merge_request_comment -> for editing a MR note
port from comments to newer 'notes' -> to get back the id

Thank you. Can you fix the tests?

master rebased
tests should pass now
empty line removed (as suggested)

hope you can :octopus:  this

@NARKOZ damn i missed the last tab - fixed now - sorry for that.

Thank you.

@hjanuschka thanks for this change! Do you know if the delete action on this endpoint is available in Gitlab? I'm getting a 405 for deletion of a "comment" I created, and I can't find mention of a delete action in the gitlab source. PUT is working though.

I don't see any way to delete one either.

@RobinDaugherty @asedge you both are right there seems to be now way to delete a MR-note via API.
i opened an issue on gitlab https://gitlab.com/gitlab-org/gitlab-ce/issues/14944.
btw. i have no clue why i added the delete - when there is no gitlab functionallity - anyway lets hope that gitlab does the change - and the gem automagically will work :).

ok its in the gitlab-upstream 👏  - seems to be released in 8.7 or so!
https://gitlab.com/gitlab-org/gitlab-ce/issues/14944


feat: implement theme changing feature to sync with parent app

This PR adds support for receiving and applying theme changes broadcasted from the parent application, ensuring a consistent look and feel across the entire application suite.
Changes

Updated the child app to listen for theme change events from the parent app via window events.
Implemented logic to dynamically update the app's theme based on changes broadcasted from the parent app.

Implemented changes same as Cadt-ui.


⚠ Lucas Boh has degraded performance

In b444833, Lucas Boh (https://lucasboh.com) experienced degraded performance:

HTTP code: 503
Response time: 28290 ms

Resolved: Lucas Boh performance has improved in 882c625.


project.root property is "overridden" by windows.first.root

Describe the bug
When using version 3.2.1, if windows.first.root is defined, it overrides the project.root configuration - starting new windows with tmux will start the shell at windows.first.root instead of project.root.
I believe this is caused by the recently added -c in tmux new-session (see the output of tmuxinator debug below) in version 3.2.1.
I have compared the behaviour to 3.2.0, and in 3.2.0 it works as I expect (the culprit is possibly #914).
In 3.2.0 the windows.first.root used cd, which only affected the shell itself. Whereas -c changes the tmux session.
I have found a workaround for myself: detach the tmux session and reattach to it using -c <project.root> arg.
Integrated this with an on_project_exit hook, e.g. on_project_exit: tmux attach-session -t bug -c ~/parent
To Reproduce
On version 3.2.1, use the following config:
name: bug
root: ~/parent

windows:
  - dir2:
      root: ~/parent/dir2
  - dir1:
      root: ~/parent/dir1

Start the session:
mkdir -p ~/parent/dir{1,2}
tmuxinator start sample

Create a new window in tmux. The current working directory is set to ~/parent/dir2 instead of ~/parent.
Expected behavior
When creating new windows in the given session, I expect the new window to be created at the path ~/parent.
This however does not happen; the new window is created at ~/parent/dir2.
Environment

tmuxinator Version: tmuxinator 3.2.1
tmux Version: tmux 3.4
Ruby Version: ruby 3.0.6p216 (2023-03-30 revision 23a532679b) [x86_64-linux]
OS: Arch Linux

Additional information
Output of tmuxinator debug bug using 3.2.1:
#!/bin/zsh

# Clear rbenv variables before starting tmux
unset RBENV_VERSION
unset RBENV_DIR

tmux start-server;

cd /home/user/parent

# Run on_project_start command.

  # Run pre command.

  # Run on_project_first_start command.

  # Create the session and the first window. Manually switch to root
  # directory if required to support tmux < 1.9
  TMUX= tmux new-session -d -s bug -n dir2 -c /home/user/parent/dir2

  # Create other windows.
  tmux new-window -c /home/user/parent/dir1 -t bug:2 -n dir1

  # Window "dir2"

  # Window "dir1"

  tmux select-window -t bug:1
  tmux select-pane -t bug:1.1

  if [ -z "$TMUX" ]; then
    tmux -u attach-session -t bug
  else
    tmux -u switch-client -t bug
  fi

# Run on_project_exit command.

Output of tmuxinator debug sample using 3.2.0:
#!/bin/zsh

# Clear rbenv variables before starting tmux
unset RBENV_VERSION
unset RBENV_DIR

tmux start-server;

cd /home/user/parent

# Run on_project_start command.

  # Run pre command.

  # Run on_project_first_start command.

  # Create the session and the first window. Manually switch to root
  # directory if required to support tmux < 1.9
  TMUX= tmux new-session -d -s bug -n dir2
  tmux send-keys -t bug:1 cd\ /home/user/parent/dir2 C-m

  # Create other windows.
  tmux new-window -c /home/user/parent/dir1 -t bug:2 -n dir1

  # Window "dir2"

  # Window "dir1"

  tmux select-window -t bug:1
  tmux select-pane -t bug:1.1

  if [ -z "$TMUX" ]; then
    tmux -u attach-session -t bug
  else
    tmux -u switch-client -t bug
  fi

# Run on_project_exit command.

Thank you for reporting this and for the detailed report!
I reverted #914 locally, and it seems to fix the issue.
Instead of just reverting that change, I've attempted to cut down on the cd commands leveraging -c while also fixing the session path.

The fix for this will be in the next release - thank you again for reporting it! 🎉

Thank you @akofink for taking care of it!


TypeError: img should be PIL Image. Got <class 'torch.Tensor'>

Hi, thanks for sharing the code.
But I met some errors when running the code to Setup KITTI Odometry. It seems that 'out = self.transform(out)' may cause the error. Below is the error message.
Dataset KITTIOdometry
    Number of datapoints: 19130
    Root location: ./data/data_odometry_velodyne/dusty-gan/sequences
  0%|                                                                                | 0/299 [00:00<?, ?it/s]
Traceback (most recent call last):
  File "process_kitti.py", line 219, in <module>
    angles = compute_avg_angles(loader)
  File "process_kitti.py", line 145, in compute_avg_angles
    for item in tqdm(loader):
  File "/root/miniconda3/envs/dpm-pc-gen/lib/python3.7/site-packages/tqdm/std.py", line 1178, in __iter__
    for obj in iterable:
  File "/root/miniconda3/envs/dpm-pc-gen/lib/python3.7/site-packages/torch/utils/data/dataloader.py", line 363, in __next__
    data = self._next_data()
  File "/root/miniconda3/envs/dpm-pc-gen/lib/python3.7/site-packages/torch/utils/data/dataloader.py", line 989, in _next_data
    return self._process_data(data)
  File "/root/miniconda3/envs/dpm-pc-gen/lib/python3.7/site-packages/torch/utils/data/dataloader.py", line 1014, in _process_data
    data.reraise()
  File "/root/miniconda3/envs/dpm-pc-gen/lib/python3.7/site-packages/torch/_utils.py", line 395, in reraise
    raise self.exc_type(msg)
TypeError: Caught TypeError in DataLoader worker process 0.
Original Traceback (most recent call last):
  File "/root/miniconda3/envs/dpm-pc-gen/lib/python3.7/site-packages/torch/utils/data/_utils/worker.py", line 185, in _worker_loop
    data = fetcher.fetch(index)
  File "/root/miniconda3/envs/dpm-pc-gen/lib/python3.7/site-packages/torch/utils/data/_utils/fetch.py", line 44, in fetch
    data = [self.dataset[idx] for idx in possibly_batched_index]
  File "/root/miniconda3/envs/dpm-pc-gen/lib/python3.7/site-packages/torch/utils/data/_utils/fetch.py", line 44, in <listcomp>
    data = [self.dataset[idx] for idx in possibly_batched_index]
  File "/userhome/dusty-gan-main/datasets/kitti.py", line 86, in __getitem__
    out = self.transform(out)
  File "/userhome/dusty-gan-main/datasets/kitti.py", line 74, in transform
    v = TF.resize(v, self.shape, Image.NEAREST)
  File "/root/miniconda3/envs/dpm-pc-gen/lib/python3.7/site-packages/torchvision/transforms/functional.py", line 319, in resize
    raise TypeError('img should be PIL Image. Got {}'.format(type(img)))
TypeError: img should be PIL Image. Got <class 'torch.Tensor'>

I found TF.resize() in torchvision<0.8.0 cannot accept torch.Tensor. What's a version of your installed torchvision?

Hi, thanks for the reply. I have corrected the version of my torchvision.


Make python version overridable by PYTHON variable

Current version builds both python versions if available. Failures are
not fatal. Make it able to choose which version to compile, install only
single version. Or no python code installed, if PYTHON= variable is
passed.

Just a note to say I'm busy doing other things at the moment but will get back to this within a week or so.

I think is good idea to add variable PYTHON3 also

The idea of PYTHON variable is to use it like PYTHON=python3 or PYTHON=python2. I am not sure what should PYTHON3 variable do.


Tree isn't folding out correctly when selecting an item from the search

Steps to reproduce

Search for a concept
Select concept from the search results

Expected behaviour
The tree should expand to the position of the selected concept.
Actual behaviour
Only the first level gets expanded.
Server configuration
Operating system:
Web server:
Database:
PHP version:
Lumen Version:
ThesauRex version:
List of activated modules:

Module List

Are you using external storage, if yes which one: local/ftp/aws/...

Actually it isn't working!


add datasource to pull all available log fields

Current Terraform version
0.14.8
Description
is it possible to add a datasource to pull all the log fields?
they are available here:
https://api.cloudflare.com/client/v4/zones/<zone_id>/logpush/datasets//fields.
Use cases
in order to properly construct the logpush job that is up to date with the fields available, would be nice to have it as a datasource
Potential Terraform configuration
none
References
No response

hi @jacobbednarz, which upstream library needs to be updated for this?
using null provisioner with local exec to get this is very fragile

the terraform provider uses cloudflare-go under the hood for the API calls; it doesn't implement anything itself. so before we can do anything here, that library needs it.

cloudflare-go should support the required api calls now that it supports account-level logpush jobs via GetAccountLogpushFields and GetZoneLogpushFields.  I'll probably try to get around to working on a PR this week unless someone else works on it.

fwiw, i've also requested that logpush/logpull simply support fields=ALL or similar to remove this requirement entirely when you just want everything... but who knows if/when that'll ever get implemented.

This issue has been closed as we are now tracking this internally with service teams directly. If you would like an update or to be notified when/if the product ships with this change, please reach out to Cloudflare Support or your account team who can watch the internal feature request for you.


Can you provide train_network.py with --stop_text_encoder_training?

Pull requests has a related commit that I tried, but at least when training the sdxl‘s lora I get
AttributeError: 'list' object has no attribute 'requires_grad_'
If such a need is not too common. I'd also appreciate it if someone here could tell me if this can be achieved with simpler modifications.
This way I don't have to use "prodigy" (because I don't have to care about the learning rate, but I can't specify different learning rates for TE and Unet) in two steps network_train_text_encoder_only, network_train_unet_only to complete 1 lora training too.

Well, it's everything related to sdxl that gets 'list' object has no attribute 'requires_grad_'


fix changing pointType

Fix #480

Warnings

:warning:

Source/dom/models/CurvePoint.js changed, but not:

📚 its docs
🧪 its tests

That's OK as long as you're refactoring.

  Generated by :no_entry_sign: dangerJS against 9c721cb4c0d3fca886692635bfbe1438552a16e9


Automated cherry pick of #2766: fix: 不能无限制追加 application/json

Cherry pick of #2766 on release/2.12.
#2766: fix: 不能无限制追加 application/json

Codecov Report

:exclamation: No coverage uploaded for pull request base (release/2.12@477ba41). Click here to learn what that means.
The diff coverage is 100%.

@@              Coverage Diff               @@
##             release/2.12   #2767   +/-   ##
==============================================
  Coverage                ?   7.22%
==============================================
  Files                   ?     472
  Lines                   ?   74525
  Branches                ?       0
==============================================
  Hits                    ?    5384
  Misses                  ?   68689
  Partials                ?     452

Flag
Coverage Δ

#aFlag
7.22% <100%> (?)

Impacted Files
Coverage Δ

pkg/util/httputils/httputils.go
47.51% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 477ba41...443c5f3. Read the comment docs.

/lgtm

/approve


Use snake case for Events range params

Description
Use snake case for range params in the Events API. This makes Events consistent with the rest of our API.
Documentation
Does this require changes to the WorkOS Docs? E.g. the API Reference or code snippets need updates.
[ ] Yes

If yes, link a related docs PR and add a docs maintainer as a reviewer. Their approval is required.

Codecov Report

Merging #200 (f8d1c47) into main (f96fcd1) will not change coverage.
The diff coverage is n/a.

:exclamation: Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.
@@           Coverage Diff           @@
##             main     #200   +/-   ##
=======================================
  Coverage   96.97%   96.97%
=======================================
  Files          46       46
  Lines        1025     1025
=======================================
  Hits          994      994
  Misses         31       31

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more


Big NPC's can diagonally walk through blocked tiles

https://github.com/user-attachments/assets/8f2b8045-4870-4f82-ba7d-9d3969e98cc7

Nice find & good timing as well, I got you fixed with our new and improved pathfinder:

Wow perfect! Good work


Memory Errors with stemcell 1.351+

In the past days we saw memory errors with the newer stemcells 1.351+. Let's use this ticket to document all findings and decide how to mitigate this issue.
Slack discussion: https://cloudfoundry.slack.com/archives/C02HWMDUQ/p1707492827160649

Some of this is in the slack thread above but here is our timeline, versions, and observations.
Tech details:  IaaS AWS Govcloud - Bosh directors at 280.0.14 and stemcell 1.351.  Diego-cells are m5.2xlarge and using a memory overallocation of 50 gb vs the physical ram of 32 gb.  The platform has used this configuration for a few years now without issue.
Back on Jan 13/14th we started to see errors in /var/log/kernel/log for Memory cgroup out of memory by OOM on deigo-cells.  At the time we where on stemcell 1.329 which was linux kernel 6.2.0-39 and cf-deployment v35.3. This deployment combo was done on Dec 28th of 2023.
Jan 23rd we moved to stemcell 1.340 which was still on linux kernel 6.2.0-39 and cf-deployment v37.0.0.  The memory cgroup out of memory errors continued in the kernel.log.  At this time we had no reported issues from our users.
Jan 30th we moved to stemcell 1.351 which was now linux kernel 6.5.0-15 and cf-deployment v37.2.0.  The memory cgroup out of memory errors continued in the kernel.log but increased.  At the same time our users started to  see OOM errors while staging their apps as well as an increase in running instances crashing with OOM errors as well - some users where seeing Instance became unhealthy: Liveness check unsuccessful: failed to make TCP connection to <silk cidr addr>:8080: dial tcp <silk cidr addr>:8080: connect: connection refused  (out of memory); process did not exit in cf events.  These crash events seemed to be spread out, not localized to specific diego-cells and happened on apps using various buildpacks. Looking at Prometheus, the value firehose_value_metric_bbs_crashed_actual_lr_ps increased as well.
Feb 1st we kept the same versions as Jan 30th but we increased deigo-cell capacity by 10%. No change in errors or reported issues.
Feb 7th we deployed cf-deployment v37.3.0 but where still on stemcell 1.351.  No change in increased kernel.log errors and user reported issues with OOM.
Feb 8th we deployed stemcell 1.360 which was linux kernel 6.5.0-17 and was suppose to contain the fix noted here.  This deployment was also done with cf-deployment v37.4.0.  Again no changes to the amount of kernel.log errors and staging errors but crashing instances increased, leveled out, but are still elevated in firehose_value_metric_bbs_crashed_actual_lr_ps.
On Feb 13th we updated our deployment to increase staging memory limit from the default 1024 to 2048.  We also expanded our diego-cell capacity another 10%.  Versions of stemcell and cf-deployment stayed the same as Feb 8th.  The kernel.log errors stayed the same.  For users that did a cf restage of their apps - most stopped getting OOM errors but a few still to happen.  We are still looking closer but the consistent failing one is based on the node-js buildpack.   The number of instances still crashing did drop looking at the firehose_value_metric_bbs_crashed_actual_lr_ps metric but still elevated from before the event.  When looking at one of the new crashes, we found it was on a new/just added diego-cell which only had 10 running instances on it and this app instance ran and crashed twice on this cell with the same kernel.log errors.
During this whole event, looking at Prometheus metrics from BOSH, the diego-cell average physical memory usage stayed around 50-65% - typical for what we have seen in the past.  No metrics, log entries, BOSH HM events indicate any of the diego-cells ran out of physical ram.   Prometheus metrics on diego-cellcapacity ofallocatedvsavailablememory showed we where between 50-70% ofallocated` in use.  The first capacity add of cells was to lower that amount closer to 50%.  The second add was to see if some additional cushion would help - it didn't.
The change of stemcell kernel from 6.2 series to 6.5 seems to have made the problem a lot worse.  Increasing staging memory is a band aid for staging apps but not the increase in crashing instances with OOM errors.
What we have not narrowed down yet was why the start of the kernel.log errors on Jan 13/14 when we were still on stemcell 1.329 using kernel 6.2.0-39.  The bug fix from above noted having to roll back to kernel 6.2.0-35 so maybe 6.2.0-39 had a bug as well but took longer over time to manifest ?  Going back over 6 months there are zero hits on these errors in our log archive.  Another CF user reported rolling back to stemcell 1.340 helped to remove most of their issues but going back to 1.340 from 1.360 re-opens at least 1 high, 2 high/med, 4 med/high CVEs according to release notes.

Dear Colleagues,
CF Foundation versions used:
CF-Deployment v37
linux stemcell v1.351, tested also on v1.360
From Cloud Foundry side we noticed the following symptom affecting CF apps.
During CF staging process we observed a large number of apps (using different buildpacks) failing with OOM.
   Exit status 137 (out of memory)
   Cell f8e8a121-82a1-4d0a-a98e-32aa29ae483d stopping instance 23a3b93a-2ce2-4470-89e5-7c89fffd3508
   Cell f8e8a121-82a1-4d0a-a98e-32aa29ae483d destroying container for instance 23a3b93a-2ce2-4470-89e5-7c89fffd3508
Error staging application: StagingError - Staging error: staging failed
FAILED

Based on our investigation we validated that the current staging container mem limit 1024 MB is not sufficient enough to stage an application which previously have been staged successfully.
In order to workaround the issue we:

We noticed that in some cases increasing the requested app memory soles the issue. I.e. Dummy hello-world application which usually requires 500 MB constantly was failing to stage. By increasing the requested mem to 1 GB the app was able to stage. Unfortunately this solution does not work in general and isn't acceptable for customers.
Had a plan to adjust a bit the general configuration for staging container size: https://github.com/cloudfoundry/capi-release/blob/96fda367a817aaccbfc4c735db0ab81882066a5c/jobs/cloud_controller_ng/spec#L927. By doing this most probably the problem would have been solved but actually the main issue should still be in place.
Decided to downgrade the stemcell to 1.340 as a last known good. This resolved in general the staging issues.

Any new updates from any of the working groups or new band-aids folks have found ?
Stemcell 1.379 was released the other day but the kernel bump is minor to 6.5.0-18 so i'm not expecting much if any changes to the issue.  We still plan to roll that stemcell out into production.
Just for reference for folks rolling back to 1.340 or older stemcells, the switch of the kernel from 6.2.X to 6.5.X was due to the 6.2.X kernel is now EOL - see: https://ubuntu.com/about/release-cycle#ubuntu-kernel-release-cycle. Rolling back to something on EOL and losing patched CVEs is a deal breaker for us.

FYI, Lakin and I have discovered that the OOMs occur with total_cache bumping up against the 1GB memory limit of the cgroup. It appears to be a problem with cache eviction not working properly. We have 25 OOMs on a 8-core 16 GB 1.379 vSphere Diego cell, and the total cache ranges from 925 MiB to 972 MiB. On the earlier stemcells, total footprint rarely exceeds 500 MiB.

Synopsis: We're still troubleshooting the OOM problem.

2/27: We've managed to get an OOM on a regular Jammy VM (not a stemcell). This helps Canonical replicate the problem (they don't need to stand up an entire Cloud Foundry foundation); however, there are some caveats:

We only saw 2 OOMs over the course of several hours on the vanilla Jammy VM; that stands in stark contrast to ~17/hour on the Diego VMs — we're missing something.

2/26 Reviewed the change in the Linux kernel's mm/memcontroller.c in the Linux kernel from v6.2 → v6.5 hoping to glean some understanding of what might be causing the OOMs on v6.5. I came up empty-handed.
2/25 We managed to replicate the problem on a 4-vCPU system; we don't need an 8- or 16-vCPU system.
2/25 Unsuccessfully attempted to eliminate the OOMs by using the sysctl job in the os-conf BOSH release by setting vm.swappiness and vm.dirty_background_ratio
2/25 Unsuccessfully attempted to eliminate the OOMs by "pre-heating" the Diego Cell (consuming the free RAM and then releasing) in a BOSH pre-start.
2/24 We managed to replicate ~100 OOMs over the course of several hours on two Diego VMs.

Synopsis:

3/1 Have not heard back from Canonical.
2/29 We were able to enhance our OOM-replication for Canonical to troubleshoot — on a regular Jammy VM, we were able to OOM almost every time. We accomplished this by lowering the memory limit from 1GiB to 512MiB. This makes it easier for Canonical to debug.
2/29 We were able to confirm that OOM does not occur on v2 cgroups (CF uses v1 cgroups). This indicates the problem may be a v1 cgroups + 6.5 kernel problem.

FAQ: OOM Errors on New Jammy Stemcells During Staging
The most-recent set of stemcells (Jammy 1.351+) have introduced intermittent OOM (out-of-memory) failures when staging Cloud Foundry applications. Though intermittent, these errors have disrupted user updates and triggered several open issues. We believe it’s a kernel bug. Until the issue is fixed, we recommend users pin to an earlier stemcell (1.340) or increase the staging limit of applications.
What’s the error that users are seeing?
Before running an application on Cloud Foundry, and application must be “staged” (choosing the appropriate buildpack (Ruby, Golang, etc.), compiling, resolving dependencies). When a user runs “cf push” or “cf restage”, it would fail with “Error staging application: StagingError - Staging error: staging failed”. When viewing the /var/log/kern.log on the Diego cell where the app was staged, one would see the error “Memory cgroup out of memory: Killed process …” along with a stack trace.
How Can I Avoid OOMs on my Foundation?
One way is to pin the stemcell to Jammy 1.340 and not upgrade past that. If that’s not possible, bump the staging RAM limit from 1 GiB to 2 GiB or higher, depending on your staging footprint. Specifically, modify dea_next.staging_memory_limit_mb. Current default is 1024; we recommend bumping it to 2048 or 4096.
Will Increasing the Staging RAM Limit Adversely Affect the Foundation?
We doubt increasing the staging RAM limit will have a negative impact unless the user is in the habit of restaging all their applications at the same time. The staging cycle is short-lived, and though staging an app will reserve a greater amount of RAM, that RAM will be released when the staging cycle completes within a few minutes.
What Stemcells are Affected?
The Linux stemcells based on Canonical’s Ubuntu Jammy Jellyfish release are affected, beginning with version 1.351 (released January 29, 2024) to 1.390 (present) are affected. That coincides with Canonical’s introduction of the 6.5 Linux kernel (prior stemcells had the 6.2 Linux kernel).
Which IaaSes are Affected?
We have seen the problem on vSphere, GCP, and AWS, and we suspect they occur on all IaaSes.
What’s Causing the Error?
We believe that the error is caused by a poor interaction between the Linux 6.5 kernel and v1 cgroups. The Linux 6.5 kernel was introduced with the 1.351 stemcell.
Which Applications are Affected?
Golang, and we have reports of NodeJS and Java as well.

2/29 We were able to enhance our OOM-replication for Canonical to troubleshoot — on a regular Jammy VM, we were able to OOM almost every time. We accomplished this by lowering the memory limit from 1GiB to 512MiB. This makes it easier for Canonical to debug.

Hi @cunnie, I ran main.go on 5.15, 6.2 and 6.5, and found that under 512MiB, we OOM every time, there is no scenario that this is enough memory to compile main.go. I also made an unbounded cgroup to see how much memory is consumed at the peak, and found it to be somewhere around 1-1.1gb needed.
$ uname -rv
5.15.0-97-generic #107-Ubuntu SMP Wed Feb 7 13:26:48 UTC 2024
$ cat /sys/fs/cgroup/memory/system.slice/512mb/memory.max_usage_in_bytes
536883200
$ cat /sys/fs/cgroup/memory/system.slice/unbounded/memory.max_usage_in_bytes
1091538944

$ uname -rv
6.2.0-39-generic #40~22.04.1-Ubuntu SMP PREEMPT_DYNAMIC Thu Nov 16 10:53:04 UTC 2
$ cat /sys/fs/cgroup/memory/system.slice/512mb/memory.max_usage_in_bytes
536920064
$ cat /sys/fs/cgroup/memory/system.slice/unbounded/memory.max_usage_in_bytes
1083924480

$ uname -rv
6.5.0-21-generic #21~22.04.1-Ubuntu SMP PREEMPT_DYNAMIC Fri Feb  9 13:32:52 UTC 2
$ cat /sys/fs/cgroup/memory/system.slice/512mb/memory.max_usage_in_bytes
537112576
$ cat /sys/fs/cgroup/memory/system.slice/unbounded/memory.max_usage_in_bytes
1180794880

Have you managed to compile main.go within a 512MiB limit on any kernel?
Do you have an example workload that used to work under 6.2, but now fails on 6.5?
I am also reviewing all commits in cgroup v1 between 6.2 and 6.5. Support for v1 does indeed exist under jammy, but v2 will always be better tested as it is more widely used, since it has been the new default for a few years now.
Thanks,
Matthew

Hey @matthewruffell
I was just trying to reproduce again and had some troubles getting it to build on 6.2, which is weird because I'm pretty sure I'm using the exact same steps I was doing the other day.
I did get it to pass on 6.2 and fail on 6.5 when turning the GOMAXPROC down to 4 from 16 which we were using the other day.
$ uname -rv
6.2.0-39-generic #40~22.04.1-Ubuntu SMP PREEMPT_DYNAMIC Thu Nov 16 10:53:04 UTC 2
$ cat /sys/fs/cgroup/memory/system.slice/oom-test/memory.limit_in_bytes
536870912
$ cat /sys/fs/cgroup/memory/system.slice/oom-test/memory.memsw.limit_in_bytes
536870912
$ GOMAXPROCS=4 ../go/bin/go build -mod vendor -ldflags="-s -w" -a .
$

$ uname -rv
6.5.0-1014-gcp #14~22.04.1-Ubuntu SMP Sat Feb 10 04:57:00 UTC 2024
$ cat /sys/fs/cgroup/memory/system.slice/oom-test/memory.limit_in_bytes
536870912
$ cat /sys/fs/cgroup/memory/system.slice/oom-test/memory.memsw.limit_in_bytes
536870912
$ GOMAXPROCS=4 ../go/bin/go build -mod vendor -ldflags="-s -w" -a .
github.com/jackc/pgtype: /workspace/go/pkg/tool/linux_amd64/compile: signal: killed
github.com/redis/go-redis/v9: /workspace/go/pkg/tool/linux_amd64/compile: signal: killed
$

We tested on the 5.15 non-hwe kernel and were not able to reproduce the issue there. So it doesn't appear to be introduced by something that got back-ported to 5.15.

Is anything going to be done for when we are back on a 6.5 kernel?

If it is a cgroups v1 & 6.5 kernel problem, the current plan is to move the noble stemcell to cgroups v2, so it should get impacted there.
It's also likely the problem will get fixed in the 6.5 kernel at some point, it's just a question of when.

The candidate stemcells with the 5.15 kernel seem to work just as well as the 6.5 ones. The current plan, unless people find problems, is to publish them on Monday.
The version numbers are obviously not what will be released, that's just the way this part of the pipeline works.
We did some testing against public IaaSs to verify VM types work. We did not do an exhaustive test of all types, but we generally found that VM types that work with the current Jammy seem to work fine with the 5.15 version too.
https://storage.googleapis.com/bosh-core-stemcells-candidate/google/bosh-stemcell-210.892-google-kvm-ubuntu-jammy-go_agent.tgz
https://storage.googleapis.com/bosh-core-stemcells-candidate/aws/bosh-stemcell-210.892-aws-xen-hvm-ubuntu-jammy-go_agent.tgz
https://storage.googleapis.com/bosh-core-stemcells-candidate/azure/bosh-stemcell-210.892-azure-hyperv-ubuntu-jammy-go_agent.tgz
https://storage.googleapis.com/bosh-core-stemcells-candidate/vsphere/bosh-stemcell-210.892-vsphere-esxi-ubuntu-jammy-go_agent.tgz
https://storage.googleapis.com/bosh-core-stemcells-candidate/openstack/bosh-stemcell-210.892-openstack-kvm-ubuntu-jammy-go_agent-raw.tgz

I think the 5.15 is a good workaround for the time being. I am still looking into the 6.5 cgroups v1 issue, but it is tricky as I can sometimes successfully get the reproducer to build correctly in 6.5 with the 512MiB ram limit, making bisecting tricky due to it not being deterministic.
I will write back once I have some more information to share. I will spend today, and early next week looking into this.
Thanks,
Matthew

Ubuntu-jammy 1.404 has been cut with the 5.15 kernel. Can people experiencing the problem confirm if this resolves it?

Hi @jpalermo @cunnie,
I have been reading all changes between 6.2 and 6.5 related to cgroups and
memory control groups.
(in Linus Torvalds linux repository)
$ git log --grep "memcg" v6.2..v6.5
$ git log --grep "memcontrol" v6.2..v6.5
$ git log --grep "mm: memcontrol:" v6.2..v6.5
I came up with the following shortlist of commits which touches memory controls
within cgroups, and especially, cache eviction:
One line shortlist (Hash, Subject):
https://paste.ubuntu.com/p/dqFPP6kPHr/
Full git log:
https://paste.ubuntu.com/p/Hd2jqnw6Qw/
Now, there are about 70 commits of interest, and I can't guarantee any of them
are the culprit without testing them against a consistent 100% reproducer, which
we don't currently have, so the following is the current theory only.
The major feature that stands out is the major work done on Multi Generational
Least Regularly Used (Multi-Gen LRU).
Documentation:
https://docs.kernel.org/mm/multigen_lru.html
https://docs.kernel.org/next/admin-guide/mm/multigen_lru.html
Explanations:
https://lwn.net/Articles/851184/
https://lwn.net/Articles/856931/
News articles:
https://www.phoronix.com/news/MGLRU-In-Linux-6.1
https://www.phoronix.com/news/Linux-MGLRU-memcg-LRU
LRU is a caching concept. Least Recently Used. The "old" implementation, used
in 5.15 and 6.2, added an "age bit" that gets incremented each time the cache
line is used. The lowest bits gets evicted when we experience memory pressure.
It is pretty simple, but not very sophisticated.
The Multi-Gen LRU is quite complicated, but the idea is to group pages together
into the idea of "generations", taking into account things like spatially (how
close the page is to another frequently used page, e.g. 5 pages in a row used
for the same thing get grouped together), among other metrics.
Multi-Gen LRU was merged in 6.1, but disabled by default. The Ubuntu 6.2 kernel
has it disabled by default. The Ubuntu 6.5 kernel turns it on by default.
$ grep -Rin "LRU_GEN" config-*
config-6.2.0-39-generic:1157:CONFIG_LRU_GEN=y
config-6.2.0-39-generic:1158:# CONFIG_LRU_GEN_ENABLED is not set
config-6.2.0-39-generic:1159:# CONFIG_LRU_GEN_STATS is not set
config-6.5.0-21-generic:1167:CONFIG_LRU_GEN=y
config-6.5.0-21-generic:1168:CONFIG_LRU_GEN_ENABLED=y
config-6.5.0-21-generic:1169:# CONFIG_LRU_GEN_STATS is not set

6.2:
$ cat /sys/kernel/mm/lru_gen/enabled
0x0000

6.5:
$ cat /sys/kernel/mm/lru_gen/enabled
0x0007

0x0000 is off. 0x0007 is fully on, as per the table in:
https://docs.kernel.org/next/admin-guide/mm/multigen_lru.html
Before I go too deep into this rabbit hole, could we please try turning off
Multi-Gen LRU, and reverting back to the basic LRU as in 5.15 and 6.2, on the
6.5 kernel?

Boot into 6.5 kernel
Confirm Multi-Gen LRU on:

$ cat /sys/kernel/mm/lru_gen/enabled
0x0007

Disable Multi-Gen LRU:

$ echo n | sudo tee /sys/kernel/mm/lru_gen/enabled
n

Check it is off:

$ cat /sys/kernel/mm/lru_gen/enabled
0x0000

Make a cgroup and try reproduce the issue with main.go or real-world workload.

Please let me know if it makes any difference. If it does, then we have our
culprit, and we can study Multi-Gen LRU more, and if it doesn't, it's back to
the drawing board, and we need to get a 100% reproducer running for further
analysis.
Thanks,
Matthew

We did some testing with the Multi-Gen LRU disabled and were unable to reproduce the problem. If anybody is still running the 6.5 kernel stemcells and is able to disable it and see if that resolves the problem that would be great data to have.
You could either run it via a bosh ssh -c against the whole instance group easily, or do it via a pre-start script and os-conf release.

Hi *,
we did some testing with Multi-Gen LRU disabled on all of our diego cells.
I can confirm that afterwards the OOM problem was no longer reproducable!
Thanks for digging into it!
Sebastian

Sounds like the ubuntu-jammy 1.404 stemcell has resolved the issue by using the 5.15 kernel. Going to close now, but reopen if people see the issue again.

Hi everyone,
Just wanted to drop by with an update on the current situation.
I was reading the commits to Multi-Gen LRU between 6.5 and 6.8, and came across
commit 669281ee7ef731fb5204df9d948669bf32a5e68d
Author: Kalesh Singh<EMAIL_ADDRESS>Date:   Tue Aug 1 19:56:02 2023 -0700
Subject: Multi-gen LRU: fix per-zone reclaim
Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=669281ee7ef731fb5204df9d948669bf32a5e68d
which was particularly interesting. The symptoms are pretty much the same,
and there is a github issue [1] which describes the same sort of issues:
[1] https://github.com/raspberrypi/linux/issues/5395
Unfortunately, this commit is already applied to the 6.5 kernel, and is a part
of 6.5.0-9-generic:
$ git log --grep "Multi-gen LRU: fix per-zone reclaim"
27c7d0b93445 Multi-gen LRU: fix per-zone reclaim
$ git describe --contains 27c7d0b93445eaadfe46bcdb57dab2090e023c19
Ubuntu-hwe-6.5-6.5.0-9.9_22.04.2~128
You were testing 6.5.0-15-generic at least, so the commit is present. So we are
looking for another fix.
I checked all the recent additions of patches into 6.5, and there was actually
a lot of Multi-Gen LRU commits very recently into the Ubuntu 6.5 kernel.
$ git log --grep "mglru" --grep "MGLRU" --grep "Multi-gen LRU" Ubuntu-6.5.0-15.15..origin/master-next
commit c28ac3c7eb945fee6e20f47d576af68fdff1392a
Author: Yu Zhao<EMAIL_ADDRESS>Date:   Fri Dec 22 21:56:47 2023 -0700
Subject: mm/mglru: skip special VMAs in lru_gen_look_around()
Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c28ac3c7eb945fee6e20f47d576af68fdff1392a
commit 4376807bf2d5371c3e00080c972be568c3f8a7d1
Author: Yu Zhao<EMAIL_ADDRESS>Date:   Thu Dec 7 23:14:07 2023 -0700
Subject: mm/mglru: reclaim offlined memcgs harder
Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=4376807bf2d5371c3e00080c972be568c3f8a7d1
commit 8aa420617918d12d1f5d55030a503c9418e73c2c
Author: Yu Zhao<EMAIL_ADDRESS>Date:   Thu Dec 7 23:14:06 2023 -0700
Subject: mm/mglru: respect min_ttl_ms with memcgs
Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8aa420617918d12d1f5d55030a503c9418e73c2c
commit 5095a2b23987d3c3c47dd16b3d4080e2733b8bb9
Author: Yu Zhao<EMAIL_ADDRESS>Date:   Thu Dec 7 23:14:05 2023 -0700
Subject: mm/mglru: try to stop at high watermarks
Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5095a2b23987d3c3c47dd16b3d4080e2733b8bb9
commit 081488051d28d32569ebb7c7a23572778b2e7d57
Author: Yu Zhao<EMAIL_ADDRESS>Date:   Thu Dec 7 23:14:04 2023 -0700
Subject: mm/mglru: fix underprotected page cache
Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=081488051d28d32569ebb7c7a23572778b2e7d57
commit bb5e7f234eacf34b65be67ebb3613e3b8cf11b87
Author: Kalesh Singh<EMAIL_ADDRESS>Date:   Tue Aug 1 19:56:03 2023 -0700
Subject: Multi-gen LRU: avoid race in inc_min_seq()
Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bb5e7f234eacf34b65be67ebb3613e3b8cf11b87
All but the very last commit are introduced in 6.5.0-27-generic, which was
just released to -updates today.
These fixups get us most of the way to all the fixes available in 6.8, and I
do wonder if they help things.
Would anyone please be able to install 6.5.0-27-generic to a stemcell, and run
a real world workload through it, with Multi-Gen LRU enabled?
I would be very eager to hear the results. These patches should help with
cache eviction and not running out memory.
Thanks,
Matthew

Hi @jpalermo @cunnie,
Have you had a chance to have a look at 6.5.0-27-generic?
Thanks,
Matthew

Hi @jpalermo @cunnie,
Is it okay to assume that memory reclaim is good enough on 6.5.0-27-generic or later?
Let me know if you are still interested in 6.5. Noble has been released with its 6.8 kernel now, I hope your cgroups v2 transition is going well.
Thanks,
Matthew


Add a cert_store option

Hi,
It would be great to be able to use the gem with for instance an in-memory certificate instead of a file!
This PR add the option to use a cert_store option where it is currently possible to specify a ca_trust_path (https://rubydoc.info/gems/httpclient/HTTPClient/SSLConfig)
I would love to have your feedback on this,
Best regards,
Sulidi

@mwrock can you accept this PR ? thanks

Is this still relevant to merge?

@pcai yes, we still use this branch on production.

@pcai can you merge this ? I'm sad this isn't in today's release.

I would've, if this had test coverage. Since it doesn't, I need to do a few manual checks for sanity.
Perhaps later this week I can take a look

thanks


Automated Tfchain shop with bitcoin

A webpage where people can buy threefoldtokens with bitcoin
preliminary spec:https://github.com/threefoldtech/rivine/blob/master/specs/webshop.md

➤ sabrinasadik commented:

[ ] Check electrum  HD wallet + Define specs & possibilities ( https://github.com/threefoldtech/rivine/issues/470 ) @robvanmieghem: 6h
[ ] Design for the token webshop using bitcoin ( https://github.com/threefoldfoundation/tfchain/issues/213 ) Roel
[ ] Implementation 42h
[ ]  Ability for the btc_electrum client to create a wallet using a Master Public Key ( https://github.com/threefoldtech/jumpscale_lib/issues/254 )

➤ Rob Van Mieghem commented:
Related but not part of this story( not required for launch):

[ ] Check and document other bitcoin wallets compatible with electrum Master Public Key restore ( https://github.com/threefoldtech/rivine/issues/471 )

➤ xmonader commented:
DigitalMe application is a flask blueprint defines the APIs to serve staticfiles or communicate with gedis server via websockets using capnp messages.
Currently, Jumpscale team replacing the flask layer with lua and openresty ( https://openresty.org/en ) and adding support for json payloads with gedis server so it'll be more like JSONRPCServer ( https://json-rpc.readthedocs.io/en/latest/quickstart.html )
So the application would be

Frontend: defined in Lua/Openresty will be responsible for routing and static files
Backend: Gedis RPCServer (python)  will be responsbile application logic

➤ Rob Van Mieghem commented:
repository: https://github.com/threefoldfoundation/tftshop

➤ Rob Van Mieghem commented:
blocked until the digital me vue integration and rest api exposure is defined


Unable to open the Google link of the pre-trained weights

Unable to open the Google link of the pre-trained weights，please help me, thanks!

Do u solve it ？i had the same question.


JetBrains: Move the plugin to a new repo, sg/jetbrains

Commits; watch out for ones that affect both the old and new repo
Issues
What do I need to check if it still works