Firefox sending low rembs on stream with 30+fps

Hi @jcague @lodoyun guys!,
so we are struggling with a bug that's driving us nuts.
Environment: OSX chrome (any version) publishing, Firefox (any version) subscribing , NO simulcast.
The webcam is a Logitech c920, which in chrome is publishing at a rate of about 29/30/31 fps (640x480):

To do this we removed from the basicexample the framerate limitation constraint [10, 20] fps.
when the publisher is alone in the server we reach full bandwidth (1000 kbits, set on licode_config.js)

when a Firefox join as a subscriber, REMBS sent from firefox will drastically drop the bandwidth to ~30kbits:

if the Firefox client disconnects, everything will return back to work.
Observations:

only reproducible in OSX chrome publisher, on ubuntu 14 client everything works fine.
tried different computers and OS

So we tried these steps:

Try Firefox publisher and Firefox subscriber -> works
Try every combination of OS and chrome and Firefox version, on the same and different machines with the same webcam, (which in every os can reach 30fps).
also tried with the server being in local network or remote AWS (natively, no docker) -> no changes, same strange behaviour.

So we maybe think is a bug of chrome with 30+ fps. then we tried the same call but in p2p: f**k -> just works flawlessly
It must be something in Licode so:

Tried to completely disable every module in the pipeline, so the server acts only as a "passthru tube" -> bandwidth continues to drops.
Tried to also commenting the c++ lines that rewrites the abs send time, so they negotiate the abs send time as extension but the server doesn't rewrite nothing. this time the server is only un-encrypting the ingoing packets and re-encrypting the outgoing packets. this is the most "transparent" way that Licode can works ("I think"). -> bandwidth constinue to drops.
Tried to analyze RR from Firefox and there are no packet lost or fraction lost and there are no error in the decoding. In fact, terminating REMBS from Firefox into Licode will result in a perfect experience, so Firefox doesn't have problems decoding the stream.
Also tried to inspect Firefox log and record a sample video call, then text2pcap the result into wireshark and everything is correct, seq. number are increasing, no packet lost, no strange padding things, nothing. everything ok...
Tried older Licode version (until v3) with all the above tests and act the same -> bandwidth drops.

At this point we are kinda lost.
We cannot think of Licode is doing something to the packets as we tried to disable every processing in the pipeline. we are printing the packets and they are going out as they're coming in.
the only thing Licode is doing is maybe adding some "latency".
Note:
Only do this with ~30fps.
Tried also Logitech c270 which can also reach 30fps same issue.
Capping the getusermedia to 29 fps (which result in an effective ~25 fps input in chrome) solves the problem.
Using the integrate webcam of various macbooks (2001, 2012, 2013, 2017 and 2018) there are no problem, since they can't quire reach 30fps.
But the strangest thing is that p2p chrome ----> Firefox is working.
I swear I'll send you a webcam if needed. This is not a real problem for us since we are only using simulcast but I think that under the hood there may be something wrong that can affect other components preventing a better experience.
This probably never went to our attention since the basicexample is capped at 20fps
Can you help us thinking on something that could be possibly making Firefox to send low REMBs?
Also available for a meeting call to discuss the problem
Ps.
This is very similar to https://github.com/lynckia/licode/issues/689 but this was chrome to Licode and solved by using abs-send-time in the bandwidth estimator.

we actually found 1 clue.
the extensions ids are not mapped during sdp exchange between browsers.
I mean for chrome abs-send-time is 3 and for firefox abs send time is 4.
but when the firefox subscribe ask to licode to subscribe to chrome streams, and says "my abs-send-time is 4" the answer sdp from the server should say "no, mine is 3".
In this way it works. currently we are sending abs-send-time extension only between same browsers.
If you print a log here
https://github.com/lynckia/licode/blob/8cfe3dcf94920d6c72d61a54d9f790a9a054d97b/erizo/src/erizo/rtp/RtpExtensionProcessor.cpp#L125
and try between browsers you'll notice.
I will pr a fix asap.
In the meanwhile we cannot understand why, excluding abs-send-time from negotiation in rtp_media_config and doing all the tests above, Firefox send low rembs to chrome osx publisher with ~30 fps, instead, in p2p, also disabling here abs-send-time from negotiation, it works.
I'll be glad to hear some thoughts

wow! great work! it makes a lot of sense but discovering the issue is not straightforward, so congrats. Your PR will probably help us improving quality when using a mixing of Chrome and Firefox.
abs-send-time is really needed for bandwidth estimation. The problem is that if we don't use it all type of delays will be considered as network delays, when that's not true and undesirable. Besides, when we use it and rewrite the packet in Licode we just take into account the network path between Licode and Firefox for bwe, which is much better given that we also generate our own REMB in Licode.

also, the more CPU  the encoder uses the higher impact on the bwe (when not using abs-send-time) it will have, because there will be more interframe delays (due to CPU constraints). That's probably a reason for having low REMBs. You could test it by disabling abs-send-time in the P2P case. In the case of using Erizo it will be even worse given that we're adding something in the middle that could add more jitter.

we now have a "difficult" scenario.
We have to choose the better way to handle the translation.
The first try is, when sending the answer, to remap the extensions to reflect the publisher.
This seems to be incorrect and non RFC compliant, in fact Firefox complains about the change of ids:
ERROR:  message: DOMException: "Answer changed id for extmap attribute at level 1 (urn:ietf:params:rtp-hdrext:toffset) from 5 to 2." in baseStack at processAnswer
So: since we set the localdescription ONLY when we received the answer in our basestack, one option would be to do the munging and extension mapping BEFORE setting our local description in the subscriber based on the server answer. in this way it works straight forward.
The all problem is due to the fact that both publisher and subscriber creates offer towards the server. If the offer for the subscriber will be generated BY the server instead, it would already worked, since firefox would set as remote description the one generated by the server which reflect the one of the publisher.
(why the decision to always be the client to make the offer? just for the sake of curiosity).
The other more "clean" to me approach, would be do the rewrite of the extensions serverside for each subscriber. It is a worse approach in terms of performance since we have to rewrite every extension id of each RTP packet, but in the end is what a server have to do, right?
I'm just curios of your opinion on the best way to approach to the problem.

The other more "clean" to me approach, would be do the rewrite of the extensions serverside for each subscriber. It is a worse approach in terms of performance since we have to rewrite every extension id of each RTP packet, but in the end is what a server have to do, right?

I also think this would be the right approach. Performance shouldn't be an issue, we're already rewritting several parts of the packets with very low impact. Not sure how to do it in Erizo though, we might expose the extension Ids from the publisher to the subscribers to help them doing those translations, or we might add the extensionIds and names that are in the RTP packet to the DataPacket structure as a map (I think this approach would be easier to implement but we should take care of the memory consumption since every DataPacket would have more info attached), wdyt? Maybe you already know a better approach.

Seems we are both thinking in the same way.
Honestly, we already started implementing this one:

or we might add the extensionIds and names that are in the RTP packet to the DataPacket structure as a map (I think this approach would be easier to implement but we should take care of the memory consumption since every DataPacket would have more info attached)

To us seemed the best tradeoff between effort/performance


All new cameras: Add camera path editing for movie recording

Hatti's BF1 tools have this, and it's really cool.
Basically it should work like this:
(All quaternions are unit quaterions. We already do this, but just in case we're not, they have to be normalized)

User clicks start-camera path
There's a user key to add path nodes. In-game, when the freecam is enabled, when a path is active, and the user presses the specific key, a pathnode is added at the location of the freecam, with the quaterion it has at that location, and the location itself in worldcoords. This is stored with the path
There's an active move speed value. It's stored with a newly created path node. The speed is actually a value how many interpolations in a second are done between two path nodes. Faster speed means lower # of interpolations. See below.
When the user is done creating the path, s/he clicks a button and the path consists of the start node, end node and all nodes in between.
the user can now start 'play' to play the camera movement over the path, where the camera is moved from path node to path node, over an interpolated beziercurve.
Use DirectX math lib's quaternion slerp function to calculate new interpolated quaternions between path nodes at x intervals, where x is calculated from the move speed of start and end node of a path part. This is the camera rotation part. The coordinates are calculated from the beziercurve through the path nodes.

Calculating the speed.
As smooth play requires a ticker, we can calculate based on the time taken between frames the time needed to play y units. E.g. if we play on 30hz, and we have a 60fps framerate, we get a new tick every 2 frames. If the speed is 10, we can define e.g. that '10' means we move 10 units per tick. If the speed is 5, we move 5 units per tick. the lower the value, the more interpolations are done between two nodes in a path.
We have to come up with a formulate for this, but it looks rather straightforward. It's either a lower-boundary on the speed (the 'slowest' one can move) or a higher-boundary on the speed (the maximum one can move).
Papers / Guides
See: https://graphicsrunner.blogspot.nl/2008/05/camera-animation-part-ii.html
Also:
See: https://www.engr.colostate.edu/ECE481A2/Readings/Rotation_Animation.pdf (or equivalent)
See: https://www.scss.tcd.ie/publications/tech-reports/reports.94/TCD-CS-94-18.pdf (bezier/bspline based camera paths)
Todo
Bezier curves/spline curves for interpolation between multiple nodes on a path. See e.g. https://gamedev.stackexchange.com/questions/64529/camera-movement-with-slerp

Hey! Wanted to ask if you get to try this out in any of the new cameras you did? I'm interested in function like this for DMC5 camera, and if you could get this done, how much do you want for it?

It's a lot of work and I don't have a lot of time atm, so when I have time I'll perhaps add it, but it's not my top priority. THe lack of time is the main roadblock

Got it, thank you for fast response!

i've used the Cinetools by hattiwatti, he went to work for DICE. The Tools were simple to use to be honest. i don't have any experience with coding other than just changing some keybinds in text files.
there was an option to do slowmotion in the tools to a degree. but i presume it would only go to a certain limit because of the servers in bf1 bf4 exct.
i know of people that want to record their footage in slowmotion to have more frames in the clips they want to record. such as myself i can only do 60fps and it hurts to not be able to slow down animations. currently i can only record freeze frames and it makes video not as interesting, because of there not being any movement other than the camera movement i can get with my controller.
especially in Batman Arkham knight, it is one of my favourite games and i would have loved to do more creative cinematics in the game. There are too few options to make something good
do you think you can make such cinetools like done by hatti in future? maybe even with better features?
and if you were to have the time. how much effort and time goes into this? i dont want to annoy people asking these kind of questions like: can you update this tool or fix a bug or implement something new.

@Kasatos95 I can make tools with camera paths etc. but as I said above, it takes a lot of work and thus time and currently I don't have a lot of time so it's not going to happen soon.
Another limiter is that I often don't have access to dof/other effects in the game's engine of the particular game. For me camera paths require effects to be settable at given points so the camera can interpolate between them. Reshade would work great but one can't set reshade properties from the outside, so camera paths might work out but there won't be camera effects in it, which to me is something which makes the whole feature useless.

ah okay, thanks for making a simple explenation! i understand that you don't have much free time, also thanks for doing what you can and have done before :) @FransBouma  you make life easier!


Error with installer script

Describe the Bug
The install instructions from quick start gave me errors:

https://github.com/cloudposse/geodesic#quickstart

$ docker run --rm cloudposse/geodesic:latest-debian init | bash
Inadequate permissions to install to /usr/local/bin. Installing to /Users/glen/.local/bin.
# Installing geodesic from cloudposse/geodesic: -debian...
docker: invalid reference format.
See 'docker run --help'.

# Failed to install geodesic
# If this is not due to file system permissions, please
# check for known issues and consider reporting the failure at
#    https://github.com/cloudposse/geodesic/issues

Steps to Reproduce

Run docker run --rm cloudposse/geodesic:latest-debian init | bash
See error

Additional Context
I've fixed the generated script this way:
--- /tmp/init-v1    2024-08-22 22:17:49
+++ /tmp/init-v2    2024-08-22 22:17:05
@@ -1,6 +1,6 @@
 #!/bin/bash
 export DOCKER_IMAGE="cloudposse/geodesic"
-export DOCKER_TAG="${1:- -debian}"
+export DOCKER_TAG="${1:-latest-debian}"
 export APP_NAME=${APP_NAME:-$(basename $DOCKER_IMAGE)}
 export INSTALL_PATH=${INSTALL_PATH:-/usr/local/bin}
 export SAFE_INSTALL_PATH="$HOME/.local/bin" # per XDG recommendations

@Nuru


Fix integration child mapping validation bug

Description
Copy of #1234 for 2.10. This fix doesn't need to go in 2.11 or main right now as neither branch is using this validation code.
Issues Resolved
Also contributes to #1234
Check List

[X] New functionality includes testing.

[X] All tests pass, including unit test, integration test and doctest

[ ] New functionality has been documented.

[ ] New functionality has javadoc added
[ ] New functionality has user manual doc added

[X] Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Closing as we're not backporting to 2.10


AccountInfo.key looks to be returning an object when fed with old mainnet accounts

attached.
0.0.910 is the a/c

AccountInfo info = new AccountInfoQuery()
.setAccountId(anyaccoperatorid)
.execute(client);
   	   	    		 out.println("

 ");
   	   	    		 out.println("
Public Key                       = " + info.key +"
");

@johnda98 Can you explain what you expect to happen? These "lower number" mainnet accounts do have an object for the key. Keys are complex structures in Hedera. As shown in your image, that account has a list of keys instead of a single key.

thanks Ryan .. yes interesting.. yep higher accounts as usual the public key .. all good then,  IDK the older accounts returned an object ref.  I'll inform our client.


CHIPDeviceController grew too much in complexity over time

Problem
CHIPDeviceController.cpp is a bit over 1700 lines of code.
CHIPDeviceController.h has:

DeviceController class implementing 4 separate delegates, has 24 data members (including 4 separate "managers", 3 delegates, 3 layers, data members declarations intermixed with method declarations)
DeviceCommisioner class extends DeviceController plus one extra delegate and has 15 data members

Proposed Solution
Organize code to be more maintainable: some members seem to be separatable in terms of usage purpose (e.g. active and paired devices, admin information, server management, certificate management) and add unit tests as applicable.

At a minimum we should start and place DeviceCommisioner into a  separate h/cpp file.
Unit tests would be great, but given the large number of data members and dependencies (especially the layers) it is probably not possible


Support for appearence-based tasks?

Is it possible to use Bayesfit in the context of appearance-based tasks?

I tried to set nafc to 1 but get an error.
I can work on a PR if this is not currently supported.

Think you mean a detection task 💡
You can set the parameter estimate for gamma to be 0.0, and fix it.  Unfortunately not able to update this code base do to contractual obligations, so PRs are always welcome :)
Hope this helps!
M
=========================
WORKING EXAMPLE:

import matplotlib.pyplot as plt
import numpy as np
import bayesfit as bf

stimulus_intensity = [0.1, 0.2, 0.3, 0.4, 0.5, 0.6, 0.7, 0.8]
trial_correct = [1, 3, 4, 8, 7, 8, 10, 10]
trial_total = [10, 10, 10, 10, 10, 10, 10, 10]

data = np.array([stimulus_intensity, trial_correct, trial_total]).transpose()
param_free = [True, True, False, False]
param_ests = [None, None, 0.0, 0.05]

metrics, options = bf.fitmodel(data,
                               nafc = 2,
                               sigmoid_type = "logistic",
                               param_free = param_free,
                               param_ests = param_ests)

bf.plot_psyfcn(data, options, metrics)

Thank you for your response, I will try this solution.


Sync eng/common directory with azure-sdk-tools for PR 7346

Sync eng/common directory with azure-sdk-tools for PR https://github.com/Azure/azure-sdk-tools/pull/7346  See eng/common workflow

API change check
API changes are not detected in this pull request.


Does linear_scale_range clip the values?

@clausmichele asked:

I wanted to ask you a question about the linear_scale_range process: Does it also have to clip the data in output or not? If we set inputMin and inputMax differently from the real min and max, we won’t obtain data having outputMin and outputMax as output.

It doesn't seem to be documented so it's not clear and we need to define a behavior and document it. What do you think is the expected and most useful behavior? From the original use case (PNG/JPEG export), I think it should clip the values.

In my opinion we have two options:

Clip the data according to outputMin and outputMax
Set automatically inputMin and inputMax according to the input data

Clip the data according to outputMin and outputMax

That's the option I meant (which is basically the same as "Clip the data according to inputMin and inputMax").

Set automatically inputMin and inputMax according to the input data

I think we didn't do this as it's not always clear what the input min/max values are without reading all data upfront. So that's the reason the user needs to specify it.

Telco: Clarify that clipping should take place for input values.

A fix is available in #167, please review.


expose set of pressed keys

I wanted to iterate over all currently pressed keys, which the current API does not expose.

Would be nice to have similar for mouse buttons. @hecrj thoughts?


Tenor out of date

Tenor has released V2

@QuetzalcoatlDevelopment Thanks for pointing it out, will be re-solved in 2-3 days.


Error on em::lock in transcation

Describe the bug
Can't set pessimistic lock on write to some entity.
Stack trace
[2019-10-01 16:26:57.441 +0000] ERROR (21634 on kgoncharov-linux): ExceptionsHandler An open transaction is required for this operation ValidationError: An open transaction is required for this operation
    at Function.transactionRequired (/home/kgoncharov/Projects/PROJECT/node_modules/mikro-orm/dist/utils/ValidationError.js:56:16)
    at UnitOfWork.lockPessimistic (/home/kgoncharov/Projects/PROJECT/node_modules/mikro-orm/dist/unit-of-work/UnitOfWork.js:283:43)
    at UnitOfWork.lock (/home/kgoncharov/Projects/PROJECT/node_modules/mikro-orm/dist/unit-of-work/UnitOfWork.js:98:24)
    at EntityManager.lock (/home/kgoncharov/Projects/PROJECT/node_modules/mikro-orm/dist/EntityManager.js:103:36)
    at MessageService.<anonymous> (/home/kgoncharov/Projects/PROJECT/src/researcher/message/message.service.ts:54:16)
    at Generator.next (<anonymous>)
    at /home/kgoncharov/Projects/PROJECT/src/researcher/message/message.service.ts:20:71
    at new Promise (<anonymous>)
    at __awaiter (/home/kgoncharov/Projects/PROJECT/src/researcher/message/message.service.ts:16:12)
    at /home/kgoncharov/Projects/PROJECT/src/researcher/message/message.service.ts:52:44
    at /home/kgoncharov/Projects/PROJECT/node_modules/mikro-orm/dist/EntityManager.js:97:31
    at /home/kgoncharov/Projects/PROJECT/node_modules/mikro-orm/dist/connections/AbstractSqlConnection.js:29:35
    at /home/kgoncharov/Projects/PROJECT/node_modules/knex/lib/transaction.js:91:24
    at bound (domain.js:415:14)
    at runBound (domain.js:428:12)
    at tryCatcher (/home/kgoncharov/Projects/PROJECT/node_modules/bluebird/js/release/util.js:16:23)
(node:21634) [DEP0097] DeprecationWarning: Using a domain property in MakeCallback is deprecated. Use the async_context variant of MakeCallback or the AsyncResource class instead.

To Reproduce
Sample code:
    const report = await this.em.findOneOrFail(Report, ...);
...
    const message = new Message({report, ...
    });

    await this.em.persistAndFlush(message);
...
    await this.em.transactional(async em => {
      await em.lock(report, LockMode.PESSIMISTIC_WRITE);
      report.messagesCount++;
    });

Expected behavior
Almost the same code worked on previous version (2.7.8)
Versions
 "mikro-orm": "^3.0.0-alpha.18",

Can you try to fetch the entity again inside the transaction?
await this.em.transactional(async em => {
  const report1 = await em.findOneOrFail(Report, ...);
  await em.lock(report1, LockMode.PESSIMISTIC_WRITE);
  report1.messagesCount++;
});

This test is passing in both mysql and postgres, which looks exactly like what you do:
  test('lock supports pessimistic locking [pessimistic write]', async () => {
    const author = new Author2('name', 'email');
    await orm.em.persistAndFlush(author);

    const mock = jest.fn();
    const logger = new Logger(mock, true);
    Object.assign(orm.em.config, { logger });

    await orm.em.transactional(async em => {
      await em.lock(author, LockMode.PESSIMISTIC_WRITE);
    });

    expect(mock.mock.calls.length).toBe(3);
    expect(mock.mock.calls[0][0]).toMatch('begin');
    expect(mock.mock.calls[1][0]).toMatch('select 1 from `author2` as `e0` where `e0`.`id` = ? for update');
    expect(mock.mock.calls[2][0]).toMatch('commit');
  });

Sounds like an issue with request context to me, will try it in my project as request context issues are pretty hard to cover with tests.
You could do something like this to get around it for now:
await this.em.transactional(async em => {
  await (em as any)['unitOfWork'].lock(report, LockMode.PESSIMISTIC_WRITE);
  report.messagesCount++;
});

Can you try to fetch the entity again inside the transaction?
await this.em.transactional(async em => {
  const report1 = await em.findOneOrFail(Report, ...);
  await em.lock(report1, LockMode.PESSIMISTIC_WRITE);
  report1.messagesCount++;
});

doesn't work for me.
Anyway, I'll try the second workaround shortly
Also, in your example above lock is applied on newly created entity, though in my case Message created and persisted, and lock is set on Report which already exists and was fetched earlier.

Also, in your example above lock is applied on newly created entity, though in my case Message created and persisted, and lock is set on Report which already exists and was fetched earlier.

Yeah but the entity is persisted and flushed, and therefore managed by the EM, so it is basically the same case.
If the workaround will work, then I am pretty sure what's happening.

It doesn't, even if I try to combine it with "fetching again" workaround.
    await this.em.transactional(async em => {
      const report1 = await this.em.findOneOrFail(
        Report, ...
      );
      await (em as any)['unitOfWork'].lock(report1, LockMode.PESSIMISTIC_WRITE);
      report1.messagesCount++;
    });

throws another error on em::lock call:
ExceptionsHandler Entity Report is not managed. An entity is managed if its fetched from the database or registered as new through EntityManager.persist() ValidationError: Entity Report is not managed. An entity is managed if its fetched from the database or registered as new through EntityManager.persist()

That new error is because you are fetching it via the parent EM, instead of the inner one.

oh, my fault. let me recheck

Same error after changing this.em to just em.

So it was the problem I had in my mind, should be now fixed in 3.0.0-alpha.19.

It works now. Thanks!


🐞 - calendar-range does not mark checked item from range if items with equal range periods

Playground Link
https://stackblitz.com/edit/angular-edlp9k?file=src%2Fapp%2Fapp.component.ts
Description
calendar-range component, if "from range" same in two items, you cannot select the second item in order from top to bottom
For example:
it is April 14
by business logic month = beginning of month - today(April 14)
by business logic current quarter = beginning of April - today(April 14)
In stackblitz try to click to "Такой же период" to reproduce problem
Angular version
17.1.4
Taiga UI version
3.77.0/ (3.83.0 stackblitz)
Which browsers have you used?

[X] Chrome
[ ] Firefox
[ ] Safari
[ ] Edge

Which operating systems have you used?

[X] macOS
[ ] Windows
[ ] Linux
[ ] iOS
[ ] Android

Duplicate of #7550

Duplicate

No, that's another problem

calendar-range component, if "from range" same in two items, you cannot select the second item in order from top to bottom

Yes, there is such a problem, because now the current active period is determined solely by dates and in the case of two identical periods there will be a collision


remove DenseFeatures from SWIG

#4463
@gf712 @avramidis @vinx13 @shubham808

Reminder for myself, first merge the gpl feature branch before this one


onNewViewStateInstance called every time when returning Fragment from backstack

There is how I put fragment in backstack
    private fun <T : Fragment> putContentFragment(fragment: T): T {
        val fragmentTag = fragment.javaClass.name
        logger.debug("putContentFragment: tag: {}", fragmentTag)

        fragmentMng.beginTransaction()
                .setTransition(FragmentTransaction.TRANSIT_FRAGMENT_OPEN)
                .replace(R.id.content_frame_layout, fragment, fragmentTag)
                .addToBackStack(fragmentTag)
                .commit()

        return fragment
    }

And there is how I navigateBack to the previous Fragment:
    override fun navigateBack() {
        if (fragmentMng.backStackEntryCount > 1) {
            callback.forceBackPressed() // it calls super.onBackPressed() for MainActivity
        } else {
            callback.finish() // it calls finish() for MainActivity
        }
    }

I also call setRetainInstance(true) on my Fragment. Class variables are restored as they must. I test it via incrementing int var every onViewCreated():
    private var someValue = 0

    override fun onViewCreated(view: View?, savedInstanceState: Bundle?) {
        super.onViewCreated(view, savedInstanceState)
        someValue++
        logger.debug("someValue: {}", someValue)
        ...
    }

    override fun createViewState() = NotesViewState()

UPD
Sometimes onNewViewStateInstance() called, sometimes not.
For example, I navigate to new fragment and then return back to the previous:

onNewViewStateInstance called only for new NoteFragment and NotesFragment was restored.
Okay. Lets restart my app and do the same:

There it is: onNewViewStateInstance called every time when returning Fragment from backstack

Yes, this is the desired behavior. Mosby saves the ViewState in the Fragment. Since the Fragment is on the backstack it can not use setRetainInstance(true). Therefore, ViewState has to be Parcelable. Hence return a ViewState implements RestorableParcelableViewState in createViewState()

Since the Fragment is on the backstack it can not use setRetainInstance(true).

But it uses. See sample in my first message. someValue increments when I navigate to other Fragment and back. ViewState must be kept too.

Class variables are restored as they must. I test it via incrementing int var every onViewCreated(): ...

I assume that in your app you are navigating from NotesFragment  to NoteFragment.
So in you example the navigation stack is:

NoteFragment (currently visible to the user)
NotesFragment (on the back stack)

Furthermore, I assume that with "restart my app and do the same" you mean that you kill the app process (or destroy hosting activity) and then restart the app so that the activity and so on are restored (Bundle in onCreate() is not null) ?
If both assumptions are correct, then this works as expected. Not sure someValue. I doubt that this  value survives restarts without putting it into a bundle.

I assume that in your app you are navigating from NotesFragment to NoteFragment.
So in you example the navigation stack is:
NoteFragment (currently visible to the user)
NotesFragment (on the back stack)

Right.
I'm not so good in English :)
I will try to rephrase. In some app sessions, ViewState is restored when I navigate to the previous Fragment from backstack. In others, it don't. someValue not stored into Bundle. It survives by setRetainINnstance(true)(but ViewState often recreates).
p.s. "restart my app" == "start new app session"

I might be wrong and something has been changed in latest support library releases but as far as I know it is not possible to put a Fragment with setRetainInstance(true) on the backstack .

I will create demo app and write as soon as I can.

Done. https://github.com/Try4W/MosbyRetainingFragmentBackstackDemo
There is no Mosby included in the project, but it proofs that "backstacked" Fragments often retained.

Thanks for this demo.
I'm wondering when this has been changed, because I remember that it was not possible to add retaining fragments to the backstack.
So you still run into the problem that onNewViewState() is called every time you come back from backstack to the previous fragment, right?
Btw. in your Activity you have to check if (savedInstanceState == null) before adding FirstFragment, otherwise your code will create a new FirstFragment on each screen orientation change.
Like this:
  @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_main);

        if (savedInstanceState == null) {
            putFragment(new FirstFragment());
        }
    }

Btw. in your Activity you have to check if (savedInstanceState == null) before adding FirstFragment, otherwise your code will create a new FirstFragment on each screen orientation change.

Yes, I forgot about it :)

So you still run into the problem that onNewViewState() is called every time you come back from backstack to the previous fragment, right?

Yeah. But sometimes onNewViewState() called like it must until I restart my app:

In some app sessions, ViewState is restored when I navigate to the previous Fragment from backstack.

It looks that there is no elegant way to update backstacked Views.
For example, I have two fragments.
The first one is ProjectFragment which holds ViewPager with multiple nested fragments.
The second is EditProjectFragment.
User open app, see ProjectFragment and navigates to EditProjectFragment
When EditProjectFragment saves Project into model, I push ProjectEdited event into my RxBus.
Then, ViewPager's fragments(presenters) handle this event and trying to update Views that is detached.
This problem can be solved via placing ViewState between View and Presenter. Like moxy does:

yes that is true ... That is more a "philosophical" question: If no View exists (or is detached), why should a presenter exist for such non existing view?
Nevertheless, I understand the problem you are facing, but I think this problem can be solved differently. No EventBus, just ensure that all presenters observe the same model.
In Mosby 3.0 I will introduce a new Model-View-Intent module. This will solve that "observing same model" problem and also remove the ViewState at all...

yes that is true ... That is more a "philosophical" question: If no View exists (or is detached), why should a presenter exist for such non existing view?

That sounds logical, yes.

In Mosby 3.0 I will introduce a new Model-View-Intent module. This will solve that "observing same model" problem and also remove the ViewState at all...

Interesting. I will read about it. When Mosby 3.0 is expected to be released? :)

... this problem can be solved differently. No EventBus, just ensure that all presenters observe the same model.

What do you mean?

I think that the easiest solution for now is prevent ViewState from retaining when the user navigates back to previous Fragment. Or how can I force ViewState to be recreated after returning to the previous Fragment from backstack?

My workaround to update only at the first start&returning from backstack:
private boolean isBackStacked = false;
private boolean isFreshInstance = true;

@Override
public void onActivityCreated(@Nullable Bundle savedInstanceState) {
    super.onActivityCreated(savedInstanceState);
    if(isFreshInstance || isBackStacked) { // fresh instance or returned from back stack
        isBackStacked = false;
        logger.debug("initializing presenter...");
        presenter.initialize();
    }
}

@Override
public void onDestroyView() {
    super.onDestroyView();
    isFreshInstance = false;
}

@Override
public void onStop() {
    super.onStop();
    if(!getActivity().isChangingConfigurations()) {
        isBackStacked = true;
    }
}

I think that better solution will be to write my own MvpDelegate but it looks very complicated to understand&edit Mosby's architecture without UML diagrams or something like that.

I had the same issue on my app. What I found was that if onSaveInstanceState was not called (eg. from screen orientation) mosby library could not correctly update the "applyViewState" flag on MvpViewStateInternalDelegate . This flag gets updated in saveViewState and in createOrRestoreViewState. The latter is not called from a retained fragment coming from backstack so the flag is not applied correctly. So I came up with the following delegate to address this issue.
/**
 * Delegate to help an {@link com.hannesdorfmann.mosby.mvp.viewstate.MvpViewStateFragment} retain its viewstate
 * when added to backstack without an orientation change.
 * When adding a fragment to backstack {@link android.support.v4.app.Fragment#onSaveInstanceState(Bundle)}
 * is not called, so the mvp library does not know that a retained viewstate exists. After the first creation we need the mvp
 * library to reset its flags in order to use the retained state with a call in {@link FragmentMvpDelegate#onCreate(Bundle)}.
 *
 * @see MvpViewStateInternalDelegate#applyViewState
 */
public class MvpViewStateBackStackSupportDelegate {

    private boolean mHasBundleOrFirstCreation;

    public MvpViewStateBackStackSupportDelegate() {
        mHasBundleOrFirstCreation = true;
    }

    public void onViewCreated(ViewState viewState, FragmentMvpDelegate mvpDelegate, Bundle savedInstanceState) {
        if (!mHasBundleOrFirstCreation && viewState != null) { //retained view state
            mvpDelegate.onCreate(savedInstanceState); //restore variables to later apply state
        }

        if (mHasBundleOrFirstCreation)
            mHasBundleOrFirstCreation = false; //reset flag, after above check
    }

    public void onSaveInstanceState() {
        mHasBundleOrFirstCreation = true;
    }

Example usage:
    private MvpViewStateBackStackSupportDelegate mBackStackSupportDelegate;

	@Override
    public void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        mBackStackSupportDelegate = new MvpViewStateBackStackSupportDelegate();
    }

    @Override
    public void onViewCreated(View view, @Nullable Bundle savedInstanceState) {
        super.onViewCreated(view, savedInstanceState);

        setRetainInstance(true);

        mBackStackSupportDelegate.onViewCreated(getViewState(), getMvpDelegate(), savedInstanceState);
    }

    @Override
    public void onSaveInstanceState(Bundle outState) {
        super.onSaveInstanceState(outState);
        mBackStackSupportDelegate.onSaveInstanceState();
    }

thanks for your feedback! I will investigate and evaluate that

Please note that I have added a Model-View-Intent (MVI) module to mosby. A sample is available in this repo. Pre release (alpha release of Mosby 3.0.0-alpha1) coming soon.  I think MVI solves this kind of problem in a more elegant way. Nevertheless, I will invest some time to find a solution for MVP too.

Fixed for MVP 3.0


DarkMode Bar Chart is always OrangeDark

I created a bar chart with the example provided and specified a custom bar chart color
BarChartView(data: ChartData(values: [("2018 Q4",63150), ("2019 Q1",50900), ("2019 Q2",77550), ("2019 Q3",79600), ("2019 Q4",92550)]), title: "Sales", legend: "Quarterly", style: Styles.barChartStyleNeonBlueLight)
It works as expected in LightMode

However, when I switch to DarkMode, the bar chart seems to always be OrangeDark

Hi there
any update here.  I'm having the same issue

Hello,
it is actually working, but you need to set it up manually.
struct TestDarkMode: View {

    var style: ChartStyle {
        let st = Styles.barChartStyleNeonBlueLight
        st.darkModeStyle = Styles.barChartStyleNeonBlueDark
        return st
    }

    var body: some View {
        BarChartView(
            data: ChartData(values:
                [("2018 Q4",63150),
                 ("2019 Q1",50900),
                 ("2019 Q2",77550), ("2019 Q3",79600), ("2019 Q4",92550)]),
            title: "Sales",
            legend: "Quarterly",
            style: style)
    }

}

var style: ChartStyle {
let st = Styles.barChartStyleNeonBlueLight
st.darkModeStyle = Styles.barChartStyleNeonBlueDark
return st
}
This should show the graph in DarkMode.

Thanks so much for the snippet! I'm relatively new to swift and was able to get my code to work with the example. I'll close this issue.


Add UForm multi-modal embeddings generator

Hey, I'd like to add Unum's UForm as a CLIP-like multi-modal embedding generator.
LMK if I should change/add something.
Thanks!

Thanks a lot your contribution!!! And the target branch should be dev

Sure, fixed :)

@ashvardanian please rebase the latest dev branch

Done, @SimFG

@ashvardanian please add a simple test case, which can use the mock server, like the openai test case

@ashvardanian i will close the pull request and change some code for the unit test case passport


cosmoz-page-location should send property updates

cosmoz-page-location should send property updates instead of full object changes

This was fixed in #19.


Set labels in the issue tracker.

i was thinking of.

shiplist
wontfix
duplicate
feature
docs

+1 go for it

Sir, Yes Sir :p


Storage integration test segfault

GCB: https://console.cloud.google.com/cloud-build/builds;region=us-east1/ed89510c-0a1f-46a5-ab14-0029de38d3c1;tab=detail?project=cloud-cpp-testing-resources
Raw: https://storage.googleapis.com/cloud-cpp-community-publiclogs/logs/google-cloud-cpp/main/bc12403ee6c20614e3adb402dd63cb11de4be8f9/fedora-36-noex/log-ed89510c-0a1f-46a5-ab14-0029de38d3c1.txt

45/58 Test #483: storage_object_integration_test .................................***Exception: SegFault  9.94 sec
Running main() from gmock_main.cc
[==========] Running 33 tests from 1 test suite.
[----------] Global test environment set-up.
[----------] 33 tests from ObjectIntegrationTest
[ RUN      ] ObjectIntegrationTest.FullPatch
[       OK ] ObjectIntegrationTest.FullPatch (617 ms)
...
...
[       OK ] ObjectIntegrationTest.GetObjectMetadataFailure (229 ms)
[ RUN      ] ObjectIntegrationTest.StreamingWriteFailure

GCB: https://console.cloud.google.com/cloud-build/builds;region=us-east1/941ecdf8-a083-4406-81c3-570da5c6e705;tab=detail?project=cloud-cpp-testing-resources
Raw: https://storage.googleapis.com/cloud-cpp-community-publiclogs/logs/google-cloud-cpp/main/e8c5586e41ba29472c865b4ac86fec714d1781e2/fedora-36-noex/log-941ecdf8-a083-4406-81c3-570da5c6e705.txt

Cannot reproduce locally.  I ran 1,000 times with ASAN and -fno-exceptions, and a few times with CMake and no exceptions.  No luck.

GCB: https://console.cloud.google.com/cloud-build/builds;region=us-east1/b629da0a-a259-4d86-8703-0e4f44771f53;tab=detail?project=cloud-cpp-testing-resources
Raw: https://storage.googleapis.com/cloud-cpp-community-publiclogs/logs/google-cloud-cpp/main/cb589628f493067c4a728a839ced623b369fa508/fedora-36-noex/log-b629da0a-a259-4d86-8703-0e4f44771f53.txt

No luck with 1,000 runs of MSAN + no-exceptions:
env -u GOOGLE_CLOUD_CPP_STORAGE_TEST_BUCKET_NAME /h/.local/bin/gunicorn --bind "<IP_ADDRESS>:7075" --worker-class sync --threads 10 --reload --access-logfile - "testbench:run()" >emulator.log 2>&1 </dev/null &

printf '{"name": "%s"}' "${GOOGLE_CLOUD_CPP_STORAGE_TEST_BUCKET_NAME}" |     curl -i -X POST --data-binary @-       -H "Content-Type: application/json"       "${CLOUD_STORAGE_EMULATOR_ENDPOINT}/storage/v1/b?project=${GOOGLE_CLOUD_PROJECT}"
CXX=clang++ CC=clang bazel test --test_env=GOOGLE_CLOUD_PROJECT=${GOOGLE_CLOUD_PROJECT} --test_env=CLOUD_STORAGE_EMULATOR_ENDPOINT=${CLOUD_STORAGE_EMULATOR_ENDPOINT} --test_env=GOOGLE_CLOUD_CPP_STORAGE_TEST_BUCKET_NAME=${GOOGLE_CLOUD_CPP_STORAGE_TEST_BUCKET_NAME} --test_env=HOME=$HOME --test_output=errors --runs_per_test=1000 --jobs=32 --config=msan --cxxopt=-fno-exceptions //google/cloud/storage/tests:object_integration_test

GCB: https://console.cloud.google.com/cloud-build/builds;region=us-east1/a76522fb-5c54-4459-8ddb-b97cc837788d;tab=detail?project=cloud-cpp-testing-resources
Raw: https://storage.googleapis.com/cloud-cpp-community-publiclogs/logs/google-cloud-cpp/9718/7af728d396424d32bb0e0eb1ebefd3b019b0d6ab/fedora-36-noex/log-a76522fb-5c54-4459-8ddb-b97cc837788d.txt

Q:  How is it that ObjectIntegrationTest.StreamingWriteFailure, which expects exceptions, is even supposed to function in a "noex" build?

Q: How is it that ObjectIntegrationTest.StreamingWriteFailure, which expects exceptions, is even supposed to function in a "noex" build?

testing_util::ExceptException<>()  expects an exception or a crash.  I am happy disabling the test for the noex build.  I am not sure if  enabling exceptions in iostreams and compiling with -fno-exceptions is UB, but it is easy to justify that it should be.
Let me try that


[amqpcpp] Fix linking amqpcpp on macOS

When you try to link your app with amqpcpp lib you'll see errors for unsupported architecture

What does your PR fix?
#19975

Which triplets are supported/not supported? Have you updated the CI baseline?

All, No

Does your PR follow the maintainer guide?

Yes

@PhoebeHui i've changed a PR description and commit changes for baseline version and new issue

Thanks @kjagosz for your updates! Could you update the version database by following steps?
vcpkg x-add-version amqpcpp
git add versions
git commit -m "Update version database"

I'm a bit concerned to turn on this switch when the upstream project has clearly indicated that they intend for it only to be used on Linux. Have you submitted an issue or documentation change request over there?

Ping @kjagosz for your response..

Closing this PR since it seems that no progress is being made. Please reopen if work is still being done.


fix: dont merge middleware config

This fixes an issue where the middleware config options were being deeply merged, so any custom middleware would still be applied alongside the original middleware, i.e. Authenticate from Filament would also be applied alongside any custom middleware in middleware.auth.

@ryangjchandler I have tested this and it resolves the issue I had.


Modify IMERG reader to ingest V07A Final products

Description
The IMERG reader was modified to read the newly released V07 products. Only Final IMERG V07A products are available at this time (August 2023), but Early and Late products are anticipated in Fall 2023. In V07 and subsequent versions, variable names have changed. Please see https://gpm.nasa.gov/sites/default/files/2023-07/IMERG_V07_ReleaseNotes_final_230713.pdf for more documentation.
Resolves #1394
Testcase
A testcase is available at /discover/nobackup/projects/lais/sharespace/imergv07_forcing_testcase

@jerlingi Thank you for the packaged testcases.  I have been able to:

compile your code with the intel compiler with strict checks.
compile your code with the gnu compiler with strict checks.
Read in IMERG V07A and replicate your testcase output.

Before merging, I would like to point @jvgeiger's attention to a few things for approval.
In  read_imerg.F90:

Line 150 and 156.  len is defined as a specific length.  Is that fine or is len=* preferred?

In readcrd_imerg.F90

Line 57 and 59.  LIS defaults to reading IMERG V06B.  Should this be changed to V07A, or left as is?  In either case, I will add an additional note in the documentation before merging.

All else looks good to me.

Thanks @cmclaug2.  I will look at that.
Also, @emkemp should look at this.  He is also reviewing IMERG V7 for our USAF project.

I agree that @emkemp should have a look too. In my opinion, we shouldn't change the default to V07A until the Early and Late versions are released (~Fall 2023) because it will fail unless Final is selected.

@cmclaug2 When you pass a string into a Fortran routine, you may declare the length of the string as (len=).  See line 148.  Fortran knows the length.  Here, the string filename was first declared in the calling routine at line 74 with a specific length (len=LIS_CONST_PATH_LEN).  So, in the called routine, you may use (len=).
For local variables, you must specify the length.  dsetname and vlname are local to read_imerghdf. So, at lines 150 and 155, a specific length is needed.

I reviewed the changes by @jerlingi.  I think the code can be improved slightly by using a formatted read '(I)' instead of the unformatted *.
However, I have more fundamental concerns about the subroutine (not @jerlingi 's fault).  Presently a return statement will be executed if any non-zero error code is returned by HDF5, without calling any HDF5 "close" subroutines to clean up under the hood.  I don't think that's acceptable -- h5close_f should always be called if there was a successful h5open_f call; h5fclose_f should be called if there was a successful h5fopen_f (to close the file!); and h5dclose_f should be called if there is a successful h5dopen_f call.
I think a subsequent pull request is in order to fix these issues.

Good catch - I agree.  One more suggestion: all processes should log status messages, not just LIS_masterproc. (Again, not @jerlingi 's fault.)

I made the '(I)' the LIS_masterproc changes.  I will look into the various h5?close_f calls.

Thanks.  I started setting up a work directory to test changes, but I've now been sucked into emergency Air Force S2S work.   I think that has the higher priority. :-(

I had some time today so I made and tested some code improvements.  Please let me know what you think.

@emkemp Your commit looks good.  I added one commit on top of yours.

What versions of IMERG do we support?
V06B: early, late, final
V07A: final

Latest code compiles, and documentation looks fine.  I accept.


Avoid mounting /var/lib/docker directory from host

As stated on this article, only the docker daemon should have exclusive access to the configuration directory, /var/lib/docker. Quoting from the article:

The Docker daemon was explicitly designed to have exclusive access to /var/lib/docker. Nothing else should touch, poke, or tickle any of the Docker files hidden there.

Why is that? It’s one of the hard learned lessons from the dotCloud days. The dotCloud container engine worked by having multiple processes accessing /var/lib/dotcloud simultaneously. Clever tricks like atomic file replacement (instead of in-place editing), peppering the code with advisory and mandatory locking, and other experiments with safe-ish systems like SQLite and BDB only got us so far; and when we refactored our container engine (which eventually became Docker) one of the big design decisions was to gather all the container operations under a single daemon and be done with all that concurrent access nonsense.

(Don’t get me wrong: it’s totally possible to do something nice and reliable and fast involving multiple processes and state-of-the-art concurrency management; but we think that it’s simpler, as well as easier to write and to maintain, to go with the single actor model of Docker.)

This means that if you share your /var/lib/docker directory between multiple Docker instances, you’re gonna have a bad time. Of course, it might work, especially during early testing. “Look ma, I can docker run ubuntu!” But try to do something more involved (pull the same image from two different instances…) and watch the world burn.

I think binding the unix socket should be enough, and the right way of doing it - by only executing docker commands and not inspecting the configuration files.

Won't fix due to retiring project


Add plumbing to implement Neon instructions with wasm simd

I also added a few functions to test out the options -- tests work on my end. Let me know if anything's wrong here.

The failed build I don't understand -- the errors seem to all be in x86 code, so I'm not sure what's wrong there. I'll look at it tomorrow.

Hey @seanptmaher , it is us. Try working off of https://github.com/nemequ/simde/commit/7b3e46778edc39ac16658e79c5fabd3c0f59cbf5 until we fix the master branch

I fixed the master branch and merged it in ; so the issue I flagged is the only build failure remaining.

If you can hold off on the implementations of NEON functions for a few days it should be in significantly better shape.  I've been working on a rewrite of the entire simde/arm directory which should be done very soon (@rosbif is waiting on this, too, so it's a high priority).

@nemequ How's the rewrite going? Please let us know if we can help in anyway. All the best, and have a good weekend!

Ignore me :) @seanptmaher pointed me to the gitter chat with your response.

This is not really applicable anymore, closing.


Gather Logs

There is some talk of standardizing a "please get me the logs for this worker" interface for the different resource managers (kubernetes, yarn, jobqueue) here: https://github.com/dask/distributed/pull/2874/files#r306986962
Is this something that we could easily implement in Dask-Jobqueue?  What form would it take?  Presumably we would find the out/err files, read their contents, and send them back to the caller.

Are these files usually made available during the job?
Is this uniform enough across deployments so that it is feasible?

Are these files usually made available during the job?

Yes at least for the job schedulers I am familiar with: log files are created when the job is submitted and updated during the job execution. Note there may be some significant delay between the execution of a print statement on the worker and the content of the log file (~1 minute even with sys.stdout.flush() on our cluster).

More details: in my experience there are some buffers here and there so that there is a significant delay between the content of the log file and what has been executed on the worker. In some experiments I ran on our cluster a while ago, even when using sys.stdout.flush() (or using PYTHONUNBUFFERED) there was a delay of ~1 minute between the time a function doing a print was submitted and the time the line reached the log file. FWIW in our cluster, using os.fsync(sys.stdout.filename()) would make the delay a lot shorter (~1 second), this may be highly cluster-specific.

Is this uniform enough across deployments so that it is feasible?

it seems feasible but is always going to be fragile ... we have the log_directory parameter that indicates where the logs end up. I reckon the jobid is in the log filename for most job schedulers by default (but of course this may not be true for some job schedulers and/or some cluster-specific setups). Note that this is always going to be a bit fragile, since the user can override the name of the logfile if he wants by using job_extra.
cc @guillaumeeb in case he has some comments about this.

With PBS Pro, you've got the qpeek $JOB_ID command to get the stdout, and qpeek -e $JOB_ID for stderr.
As far as I know, output files are not availabe until the end of the job, you need to use these commands when the job is running. But maybe you can go and find them on the node where the job is running...
Nonetheless, there is a way to get those logs. It is just already different from what @lesteve is used to...

Ahhh the wonderful world of HPC clusters, where everybody does something just slighly different than everybody else ;-)

Is there suggested best practice for dask-jobqueue for flushing worker logs on SLURM. I'm finding that i'm not getting print statements until the process is tearing down, sometimes hours later. Where should I flush to try to improve?
From the local worker directory I see lots of logs, but they are empty for long periods of time. Everything prints eventually without an issue.
cat dask-worker-49747319.out

I would guess this is not specific to dask-jobqueue but to log buffering from your job scheduler and/or your cluster filesystem so this is likely to be very cluster-specific.
I found this in my notes from a while ago on my SGE cluster. Note some of this may have performance implications:
Summary this works:

Adding in env_extra works: 'export PYTHONUNBUFFERED=non_empty'. delay ~1
minute (SGE log in my $HOME).
Using sys.stdout.flush() delay of ~1 minute (SGE log in my $HOME). Note for stderr
sys.stderr.flush() does help as well for some reason (I thought stderr was
not buffered but it might be by Python ...).
sys.stdout.flush() + os.fsync(sys.stdout.fileno()) is instant.


Consider cropping around the spinal cord

Cropping around the spinal cord for training and inference could help with:

class imbalance
training/inference time
memory issues

There are at least two approaches for cropping:

segment the cord, dilate and crop at min/max boundaries. See https://github.com/ivadomed/model_seg_ms_mp2rage/blob/61a01ed1a42a78268514b69fb6db2f52cafba0d7/preprocessing/preprocess_data.sh#L123-L125.
segment the cord, straighten by translating all axial slices, and slightly dilate, yielding a 'straight' spinal cord on which a 2D or 3D segmentation kernel can be used. See eg. https://github.com/spinalcordtoolbox/spinalcordtoolbox/blob/22c0c51d520add3a68dc8b10b37c14379312aa1e/spinalcordtoolbox/deepseg_/lesion.py#L119

pros: more agnostic to cord curvature
cons: not applicable to sagittal data, where R-L resolution is not sufficient to do this straightening.

Related to #29

I'm just adding that the SC segmentation does not work perfectly for PSIR/STIR images (i.e., contrasts with lesion masks). This should hopefully not be a problem as the SC segmentation will only be used for cropping. Here is a comment in which I investigated different settings for SC seg. Alternatively, we can bring T2w seg to PSIR/STIR, as done here.

Just pushed the modifications on branch plb/nnunet:

created seg_sc.py file to segment the spinal cord using sct
created crop_sc.py file to crop an image using a mask
modified the convert_bids_to_nnunet.py  to enable cropping when converting the dataset to the nnU-Net format.

Now when calling convert_bids_to_nnunet.py we can ask to crop the training and/or the testing images. However, this process is very long.

Cool progress, @plbenveniste!
A few notes:

I would add QC to the SC segmentation command (using -qc and -qc-subject flags) to quickly access the SC seg quality. Potentially, I would also consider QC for the cropping (sct_crop_image does not support QC natively, but maybe we could use some tweak with sct_qc)
I am not sure if sct_deepseg_sc you are using here works well for both PSIR and STIR. IIRC, sct_propseg -c t1 worked better for PSIR; see this comment.
Since both seg_sc.py as well as crop_sc.py are essentially single line functions, you could include them directly within the convert_bids_to_nnunet.py script.
I would keep the original convert_bids_to_nnunet.py script without SC seg and cropping as it is. And created a second script, e.g., convert_bids_to_nnunet_seg_and_crop.py. This allows us to reproduce both workflows, the one without cropping and the one with cropping.
This last note is more general - if you run your scripts, they will process subjects individually, which can take a long time in the case of the canproco dataset (n~450 subjects). From this perspective, it would be better to use the sct_run_batch wrapper for SC seg and cropping. The sct_run_batch allows parallelization across subjects.

Note: All my notes are just for brainstorming; I am not discarding any ideas.

Currently the cropping approach is exploratory. So we don't need a 'clean' pipeline with proper QC, manual correction, etc. Obviously, you want to make sure that the segmentation is "good enough", ie: works in >90% of your images (otherwise you could not conclude about the efficiency of cropping for model training). In order to assess this, you need QC. So that could be added in your function call:
https://github.com/ivadomed/canproco/blob/cfe61fdf18073e01f97d45f379f7188095392c8a/nnunet/seg_sc.py#L24C41-L24C74
If segmentation does not work well (because of the anisotropic and sagittal orientation), then you might need to:

manually correct some segmentation

pros: precise seg
cons: takes a LOT of time

run the segmentation on the T2w image (which is WAY more robust bc good cord/CSF contrast and isotropic resolution), and then bring that segmentation in the space of the PSIR/STIR (using sct_register_multimodal -identity 1)

pros: time efficient, robust
cons: if subject moves, segmentation is shifted. HOWEVER, in this case you only need the seg for cropping, so it's OK to have slight shift.

About using sct_run_batch: the way the functions are called (ie: system call within Python) makes it more difficult to use sct_run_batch. Now that the functions are coded, I suggest to just use them, and then conclude if cropping is a good idea or not. And if not, revert to the previous version (or do not merge to master).
That being said, it might be OK to keep the seg/crop and upgraded version of nnunet conversion script as is (ie: with the crop option)-- however it needs to be documented.

New script for segmentation of the spinal cord on an entire dataset: seg_dataset.py.
It creates new files with the suffix '_seg'.
Currently performing QC on the segmentations.

It's hard to perform QC with the axial view :

This corresponds to this:

Currently modifying the code to have sagittal view to perform quality control

Solved problem :

However, there is still one problem: PSIR images tend to have bad segmentations with the lower slices of the volume having no segmentations.
Since, in this case, we are only interested in having these segmentations for the cropping of the image, we use the obtained segmentations and dilate them on the z-axis. This way we keep the entirety of the spine while removing volumes "far" from the centerline to reduce the class imbalance in the dataset.

To have a better QC for the segmentation, we used: sct_qc -i {image} -s {output_path} -d {output_path} -p sct_deepseg_lesion -plane sagittal -qc {qc_folder}

Cool trick! In the past, I used -p sct_label_vertebrae to create sagittal SC QC. But it created only a single slice. So the workaround with -p sct_deepseg_lesion -plane sagittal is definitely better!

After all, in this case, QC is more interesting after cropping than after the segmentation of the spinal cord.
To perform QC on the cropping I used:
sct_qc -i {image} -d {output_path} -p sct_image_stitch -plane sagittal -qc {qc_folder}
Here is the result:

I ran segmentation and cropping on Joplin with the following code

os.system(f'python ./canproco/nnunet/seg_dataset.py -i ./data/canproco/ -c STIR,PSIR -q ./data/qc_seg')
os.system(f'python ./canproco/nnunet/crop_dataset.py -i ./data/canproco/ -c STIR,PSIR -q ./data/qc_seg -r True')

Segmentation on STIR was done without any problem.
However, I had the following problem when cropping on PSIR (ie. using prop_seg):

It seems that there is a problem with the location at which the output file is supposed to be. However, the folder is stored at the same location for sct_deepseg_sc and it works fine.

NB: It works fine on my computer but not on Joplin. (maybe an SCT issue)

Interesting, I just tried sct_propseg on joplin on the same subject with the latest SCT master, and the command worked for me:
terminal output
$ cd ~/extrassd1/janvalosek/data/canproco/sub-edm143/ses-M0/anat
$ sct_propseg -i sub-edm143_ses-M0_PSIR.nii.gz -c t1 -o sub-edm143_ses-M0_PSIR_seg.nii.gz

--
Spinal Cord Toolbox (git-master-0204cfebce8a005dbb8d14768270135ea32839bc)

sct_propseg -i sub-edm143_ses-M0_PSIR.nii.gz -c t1
--

Image header specifies datatype 'int16', but array is of type 'float64'. Header metadata will be overwritten to use 'float64'.
Creating temporary folder (/tmp/sct_2023-07-21_14-32-50_propseg_vw16rmuy)
Image header specifies datatype 'int16', but array is of type 'float64'. Header metadata will be overwritten to use 'float64'.
Creating temporary folder (/tmp/sct_2023-07-21_14-32-50_optic-detect-centerline_sy406lrg)
Remove temporary files...
rm -rf /tmp/sct_2023-07-21_14-32-50_optic-detect-centerline_sy406lrg
Creating temporary folder (/tmp/sct_2023-07-21_14-32-52_propseg-centerline-optic_wuvdypvo)
/home/GRAMES.POLYMTL.CA/p118175/code/spinalcordtoolbox/bin/isct_propseg -t t1 -o /tmp/sct_2023-07-21_14-32-50_propseg_vw16rmuy -verbose -i /mnt/extrassd1/janvalosek/data/canproco/sub-edm143/ses-M0/anat/sub-edm143_ses-M0_PSIR.nii.gz -init-centerline /tmp/sct_2023-07-21_14-32-52_propseg-centerline-optic_wuvdypvo/centerline_optic.nii.gz -centerline-binary # in /mnt/extrassd1/janvalosek/data/canproco/sub-edm143/ses-M0/anat
mv /tmp/sct_2023-07-21_14-32-50_propseg_vw16rmuy/sub-edm143_ses-M0_PSIR_seg.nii.gz ./sub-edm143_ses-M0_PSIR_seg.nii.gz
mv /tmp/sct_2023-07-21_14-32-50_propseg_vw16rmuy/sub-edm143_ses-M0_PSIR_centerline.nii.gz ./sub-edm143_ses-M0_PSIR_centerline.nii.gz

Check consistency of segmentation...
Creating temporary folder (/tmp/sct_2023-07-21_14-33-21_propseg-check-segmentation_dqmgdz8e)
/tmp/sct_2023-07-21_14-33-21_propseg-check-segmentation_dqmgdz8e/tmp.segmentation.nii.gz
/tmp/sct_2023-07-21_14-33-21_propseg-check-segmentation_dqmgdz8e/tmp.centerline.nii.gz

Get data dimensions...
/tmp/sct_2023-07-21_14-33-21_propseg-check-segmentation_dqmgdz8e/tmp.segmentation_RPI_c.nii.gz
rm -rf /tmp/sct_2023-07-21_14-33-21_propseg-check-segmentation_dqmgdz8e
Copy header input --> output(s) to make sure qform is the same.
Image header specifies datatype 'float64', but array is of type 'uint8'. Header metadata will be overwritten to use 'uint8'.
Image header specifies datatype 'float64', but array is of type 'uint8'. Header metadata will be overwritten to use 'uint8'.

Done! To view results, type:
fsleyes sub-edm143_ses-M0_PSIR.nii.gz -cm greyscale sub-edm143_ses-M0_PSIR_seg.nii.gz -cm red -a 100.0 &

I found the problem:
sct_propseg doesn't like it when the output is in a subfolder. If you try this, it shouldn't work normally:
sct_propseg -i sub-edm143_ses-M0_PSIR.nii.gz -c t1 -o ./test/sub-edm143_ses-M0_PSIR_seg.nii.gz

I found the problem:
sct_propseg doesn't like it when the output is in a subfolder. If you try this, it shouldn't work normally:
sct_propseg -i sub-edm143_ses-M0_PSIR.nii.gz -c t1 -o ./test/sub-edm143_ses-M0_PSIR_seg.nii.gz

Okay, sct_propseg has a problem with . in the output filename. If I use $PWD instead of ., it works:
$ sct_propseg -i sub-edm143_ses-M0_PSIR.nii.gz -c t1 -o $PWD/test/sub-edm143_ses-M0_PSIR_seg.nii.gz

Could you please open an issue about this under SCT repo?

Segmentation and cropping done.
QC on cropping: out of the 782 images, 39 had a bad segmentation (either missing some lower part of the spinal cord or segmenting something completely out of the zone of interest)

List of images to change
sub-mon009_ses-M12_PSIR.nii.gz
sub-mon032_ses-M0_PSIR.nii.gz
sub-mon050_ses-M0_PSIR.nii.gz
sub-mon055_ses-M0_PSIR.nii.gz
sub-mon096_ses-M12_PSIR.nii.gz
sub-mon107_ses-M0_PSIR.nii.gz
sub-mon113_ses-M0_PSIR.nii.gz
sub-mon118_ses-M0_PSIR.nii.gz
sub-mon129_ses-M0_PSIR.nii.gz
sub-mon138_ses-M12_PSIR.nii.gz
sub-mon142_ses-M12_PSIR.nii.gz
sub-mon148_ses-M0_PSIR.nii.gz
sub-mon150_ses-M12_PSIR.nii.gz
sub-mon152_ses-M0_PSIR.nii.gz
sub-mon168_ses-M0_PSIR.nii.gz
sub-mon175_ses-M0_PSIR.nii.gz
sub-mon185_ses-M12_PSIR.nii.gz
sub-mon191_ses-M0_PSIR.nii.gz
sub-mon193_ses-M0_PSIR.nii.gz
sub-mon209_ses-M12_PSIR.nii.gz
sub-mon036_ses-M0_PSIR.nii.gz
sub-mon202_ses-M12_PSIR.nii.gz

sub-van137_ses-M12_PSIR.nii.gz
sub-van161_ses-M0_PSIR.nii.gz
sub-van171_ses-M0_PSIR.nii.gz
sub-van176_ses-M0_PSIR.nii.gz

sub-tor057_ses-M0_PSIR.nii.gz
sub-tor139_ses-M0_PSIR.nii.gz

sub-edm105_ses-M0_PSIR.nii.gz
sub-edm142_ses-M12_PSIR.nii.gz

sub-edm075_ses-M0_PSIR.nii.gz
sub-tor133_ses-M0_PSIR.nii.gz
sub-edm142_ses-M12_PSIR.nii.gz
sub-edm156_ses-M12_PSIR.nii.gz

sub-tor103_ses-M12_PSIR.nii.gz
sub-tor133_ses-M0_PSIR.nii.gz
sub-tor079_ses-M0_PSIR.nii.gz

Improvement of the Dice score : around 0.55

By looking at the prediction results, it seems that the model is more exhaustive in predicting lesion segmentation. That is most likely due to class imbalance reduction thanks to the cropping of the volumes around the spinal cord.
Currently working on a script to detail the prediction results on the images from the M12 time point.

I noticed that some images were exactly the same between M0 and M12. Maybe that's an issue to look at ?

None exhaustive list
sub-mon003_ses-M0_PSIR.nii.gz same as M12
sub-tor029_ses-M0_PSIR.nii.gz same as M12
sub-mon011_ses-M0_PSIR.nii.gz same as M12
sub-tor016_ses-M0_PSIR.nii.gz same as M12
sub-tor021_ses-M0_PSIR.nii.gz same as M12

Good catch! Can you please open a separate issue about that under this repo? We will have to contact the sites and figure out what happed.

It seems that the model didn't have time to converge to an optimal model. Let's try on 2000 epochs.

Nota Bene:
It seems that the segmentations include more segmentations when performed with --disable_tta than when run without.
Example on image sub-cal072_ses-M12_STIR: it identifies one more lesion, another lesion is seen on a slice where it wasn't seen before, and overall lesions segmented are 'bigger'.

On the left with tta, on the right with --disable_tta

It seems that the segmentations include more segmentations when performed with --disable_tta than when run without.

very interesting observation. Tagging @naga-karthik @rohanbanerjee @tzebre


Add cEUR support to the Request flow

Design in Figma
In the request flow, the decision of which stable coin to use is up to the person who sends the money. The person who receives the money creates a request in fiat currency, the same as now.
If the person who sends money holds only one balance, their sending flow stays as it is now. If they hold 2 stable currency balances, the most recently used coin will be chosen by default and there will be an edit button on the transaction confirmation screen that will allow changing the balance.

https://valora.testquality.com requirement R9. Do not remove or edit.


[Extends #787] - Improve json conversion with tests and support for older Rails 3.x

Updates https://github.com/shakacode/react_on_rails/pull/787 with some tests.

This change is

It seems I cannot fetch the upstream to be up-to-date with the master branch.

Please resolve the conflicts a add a test with a Rails version of 10. We can't easily run CI on rails 32. If you want to submit a PR on that, that would be awesome. Contact me if you want advice on how to do that.

A couple comments. I'd like to see the escaping part refactored to it's own class for better testing.
CC: @cheremukhin23 @ynote @squadette

Reviewed 2 of 3 files at r1, 1 of 1 files at r2.
Review status: all files reviewed at latest revision, 4 unresolved discussions, some commit checks failed.

CHANGELOG.md, line 9 at r2 (raw file):

### Improved
- Improve json conversion with tests and support for older Rails 3.x. [#787](https://github.com/shakacode/react_on_rails/pull/787) by [cheremukhin23](https://github.com/cheremukhin23).

@ynote you can add you as an author as well ;-)

app/helpers/react_on_rails_helper.rb, line 229 at r2 (raw file):

  def json_safe_and_pretty(hash_or_string)
    unless hash_or_string.class.in?([Hash, String])
      raise "#{hash_or_string.class} is unsupported argument class for this method"

list what types are supported in the error message

app/helpers/react_on_rails_helper.rb, line 243 at r2 (raw file):

  def rails_version_less_than(version)
    Gem::Version.new(Rails.version) <= Gem::Version.new(version)

need tests for this this
main issue is that we originally had string comparisons and "10.0" is < "4"
In order to best test private methods like this, I'd like to see this escaping functionality moved to a different ruby class.
That will work best with @squadette's idea to pretty print in some cases. See #760.

spec/dummy/spec/helpers/react_on_rails_helper_spec.rb, line 59 at r2 (raw file):
Previously, Ynote (Little Cheung) wrote…
I'm not really sure that mocking the version is the better way to do.
Could it be a good idea to configure Travis to lauch the tests with differents Rails version? I'm just raising the question, I know this imply to extend the tests duration and to change the Travis configuration.

Yes, but it would take (see #814):

/spec/dummy-rails3 (like spec/dummy, but with Rails 3)
pass env value on travis runs for different ruby versions and ONLY run rails 3 tests (spec/dummy-rails3) and unit tests) for the old ruby version. Don't run /spec/dummy and the generator tests.

Comments from Reviewable

Review status: 0 of 7 files reviewed at latest revision, 4 unresolved discussions.

app/helpers/react_on_rails_helper.rb, line 243 at r2 (raw file):
Previously, justin808 (Justin Gordon) wrote…
need tests for this this
main issue is that we originally had string comparisons and "10.0" is < "4"
In order to best test private methods like this, I'd like to see this escaping functionality moved to a different ruby class.
That will work best with @squadette's idea to pretty print in some cases. See #760.

I moved this method into ReactOnRails::Util as it could be used elsewhere too. Is it ok for you?

spec/dummy/spec/helpers/react_on_rails_helper_spec.rb, line 59 at r2 (raw file):
Previously, justin808 (Justin Gordon) wrote…
Yes, but it would take (see #814):

/spec/dummy-rails3 (like spec/dummy, but with Rails 3)
pass env value on travis runs for different ruby versions and ONLY run rails 3 tests (spec/dummy-rails3) and unit tests) for the old ruby version. Don't run /spec/dummy and the generator tests.

Ok, I'll have a look at this later. I'll try to find some time to work on it.

Comments from Reviewable

Wow! @ynote GREAT JOB!
This is really OUTSTANDING!

Reviewed 7 of 7 files at r3.
Review status: all files reviewed at latest revision, 4 unresolved discussions, some commit checks failed.

Comments from Reviewable

@Ynote Please be sure that your PR passes CI.
You have many linter errors. Many will be fixed by running rubocop -a.

@Ynote Do you need any help from me to get this one finished up? I want to do one last 6.x release before I merge in the webpacker changes and go to 7.0.0-beta.0

Review status: all files reviewed at latest revision, 3 unresolved discussions, some commit checks failed.

lib/react_on_rails/json_output.rb, line 13 at r3 (raw file):

    def escaped
      return escaped_without_erb_utils if Utils::rails_version_less_than("4")

We need to confirm this is less than 4 and not less than 4.2, per #815  and #816 by @EdmundLeex

Comments from Reviewable

Hi,
I'm sorry, I wasn't available yesterday. I'm gonna have a look at these errors and fix it now.

Review status: all files reviewed at latest revision, 3 unresolved discussions, some commit checks failed.

lib/react_on_rails/json_output.rb, line 13 at r3 (raw file):
Previously, justin808 (Justin Gordon) wrote…
We need to confirm this is less than 4 and not less than 4.2, per #815  and #816 by @EdmundLeex

Ok, I'm gonna change the tested version number.

Comments from Reviewable

Sorry I didn't run rubocop before -_-" It is fixed now! Hope it will be ok for your release.

Review status: 3 of 7 files reviewed at latest revision, 3 unresolved discussions, some commit checks failed.

lib/react_on_rails/json_output.rb, line 13 at r3 (raw file):
Previously, Ynote (Little Cheung) wrote…
Ok, I'm gonna change the tested version number.

After readinghttps://github.com/shakacode/react_on_rails/issues/815 and https://github.com/shakacode/react_on_rails/pull/816, I suppose what you meant was to confirm this is less than 4.2 and not 4 :)

Comments from Reviewable

Coverage increased (+0.04%) to 98.02% when pulling 460eb372fabd0f345350832897a699403cdd5c04 on KissKissBankBank:improve-json-safe-and-pretty into 1f48e63cc0820c6bba2eb92ac915fc4ca7c05a53 on shakacode:master.

I have a couple of maybe naive questions.

Why not just reimplement the ERB::Util::json_escape method since its a fairly simple one instead of checking the Rails version? (this seems adds unnecessary performance overhead, mainly in doing Gem.new(...))
Maybe a class method instead of a new JsonOutput class and instantiate an instance every time? Ruby object instantiation is costly and since there is not state changes, seems creating an object is just adding performance overhead. (I brought this up because we are at a stage where performance really matter)

This method is called in every request and maybe multiple times in each request. Think we might want to pay more attention to performance.

@EdmundLeex:

Why not just reimplement the ERB::Util::json_escape method since its a fairly simple one instead of checking the Rails version? (this seems adds unnecessary performance overhead, mainly in doing Gem.new(...))

Suppose an XSS is discovered. We want to leverage what's in the Rails code.
@Ynote, @EdmundLeex:  can we do some small memoization, maybe at the class level to address @EdmundLeex's concerns. I agree with him that anything in the rendering cycle should be fairly efficient. That being said. I bet if this were profiled, there would be ZERO difference if we optimized this.
I need to get this merged and shipped this weekend, as I'm about to go beta for 7.0.0

Review status: 3 of 7 files reviewed at latest revision, 4 unresolved discussions, some commit checks failed.

lib/react_on_rails/json_output.rb, line 13 at r3 (raw file):
Previously, Ynote (Little Cheung) wrote…
After readinghttps://github.com/shakacode/react_on_rails/issues/815 and https://github.com/shakacode/react_on_rails/pull/816, I suppose what you meant was to confirm this is less than 4.2 and not 4 :)

I'm not 100% sure on the exact version to compare. This sounds reasonable!

lib/react_on_rails/json_output.rb, line 19 at r4 (raw file):

    def escaped
      return escaped_without_erb_utils if Utils.rails_version_less_than("4.2")

@ynote Let's add a method that encapsulates the 4.2 check and memoize the result.
We also can make the method static, since this class is so simple.
I believe that would address @edmundleex concerns.

Comments from Reviewable

Reviewed 4 of 4 files at r4.
Review status: all files reviewed at latest revision, 4 unresolved discussions, some commit checks failed.

Comments from Reviewable

FYI: 4.1.1 was the first version that corrected this issue.

Thanks @EdmundLeex for your concern about performance!
@justin808, I added the memoization and follow @EdmundLeex's advice using the test on Rails 4.1.1.

Review status: 3 of 7 files reviewed at latest revision, 4 unresolved discussions, some commit checks failed.

Comments from Reviewable

Coverage increased (+0.05%) to 98.03% when pulling a168047f7f9f9eba0818aa476c24d84a415e8163 on KissKissBankBank:improve-json-safe-and-pretty into 1f48e63cc0820c6bba2eb92ac915fc4ca7c05a53 on shakacode:master.

Coverage increased (+0.05%) to 98.025% when pulling 06dd57d02f70fa898a56188cb77d8f576fe4d1ec on KissKissBankBank:improve-json-safe-and-pretty into 1f48e63cc0820c6bba2eb92ac915fc4ca7c05a53 on shakacode:master.

Coverage increased (+0.05%) to 98.025% when pulling 06dd57d02f70fa898a56188cb77d8f576fe4d1ec on KissKissBankBank:improve-json-safe-and-pretty into 1f48e63cc0820c6bba2eb92ac915fc4ca7c05a53 on shakacode:master.

Coverage increased (+0.05%) to 98.03% when pulling f5feb91b7f00eefc20b31234e448876833153790 on KissKissBankBank:improve-json-safe-and-pretty into 1f48e63cc0820c6bba2eb92ac915fc4ca7c05a53 on shakacode:master.

Reviewed 3 of 3 files at r6, 2 of 2 files at r7.
Review status: all files reviewed at latest revision, 7 unresolved discussions, some commit checks failed.

lib/react_on_rails/utils.rb, line 39 at r7 (raw file):

    end

    def self.rails_version_less_than_4_1_1

was the bug fixed in 4.1.1? (rather than 4.2?)

Comments from Reviewable

@ynote Please confirm that we should merge and release.

Review status: all files reviewed at latest revision, 7 unresolved discussions, some commit checks failed.

Comments from Reviewable

I just tested the PR again with my stack. Everything is ok for me. I think we can merge and release.

Review status: all files reviewed at latest revision, 7 unresolved discussions, some commit checks failed.

lib/react_on_rails/utils.rb, line 39 at r7 (raw file):
Previously, justin808 (Justin Gordon) wrote…
was the bug fixed in 4.1.1? (rather than 4.2?)

This is the PR that fixes the eating quotes issue and the issue with \u2028 and \u2029special characters: https://github.com/rails/rails/pull/13073. It was merged in Rails 4.1. So, I think it is ok.

Comments from Reviewable

Thank you @Ynote!


python3Packages.pytorch-metric-learning: mark as broken

Description of changes
This package has been broken on hydra for ~1 year (https://hydra.nixos.org/build/174163600). Fixing it looks like it will require packaging faiss
cc @bcdarwin

Things done

Built on platform(s)

[ ] x86_64-linux
[ ] aarch64-linux
[ ] x86_64-darwin
[ ] aarch64-darwin

[ ] For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
[ ] Tested, as applicable:

NixOS test(s) (look inside nixos/tests)
and/or package tests
or, for functions and "core" functionality, tests in lib/tests or pkgs/test
made sure NixOS tests are linked to the relevant packages

[ ] Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
[ ] Tested basic functionality of all binary files (usually in ./result/bin/)
22.05 Release Notes (or backporting 21.11 Release notes)

[ ] (Package updates) Added a release notes entry if the change is major or breaking
[ ] (Module updates) Added a release notes entry if the change is significant
[ ] (Module addition) Added a release notes entry if adding a new NixOS module
[ ] (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes

[ ] Fits CONTRIBUTING.md.

The mass Python rebuilds are a bit of an issue in that every so often they will update packages I maintain - directly, not just via dependency update - breaking them in the process and then merging the broken version back into trunk.


Final docs and audit cleanup

This is the last bit of cleanup I wanted to do to close out #191. There are a few overlapping things here (let me know if I should pull any of these out):

Added more detail to CONTRIBUTING.md about required versions, manual testing and debugging, etc.
Fixed assorted typos, minor grammar things, marked up inline code snippets and symbol references as code, made issue references into links, made directory references consistent (e.g. lib/ vs. /lib), etc. in docs.
A few more dependency updates to clear all the remaining audit issues.

Nice work, thanks! These all look like great improvements :+1:


WebJob deployment fails with CODE:414

I'm using webapps-deploy@v2 action to deploy .net core web job build artifact into Azure app service.  But it's giving below error everytime.
Run azure/webapps-deploy@v2
  with:
    app-name: web-app-test-eu-xxxxxxxxxxxxx-001
    package: .
    publish-profile: ***
    slot-name: production
  env:
    AZURE_WEBAPP_NAME: web-app-test-eu-levomethanol-001
    AZURE_WEBAPP_PACKAGE_PATH: .
    DOTNET_VERSION: 3.1.100
    BUILD_CONFIGURATION: Release
    BINARIES: ./app_data/jobs/continuous/XX.Integration.xxxxxxxx.WebJobs
    JAVA_HOME_11.0.14_x64: C:\hostedtoolcache\windows\jdk\11.0.14\x64
    JAVA_HOME: C:\hostedtoolcache\windows\jdk\11.0.14\x64
    JAVA_HOME_11_0_14_X64: C:\hostedtoolcache\windows\jdk\11.0.14\x64
    DOTNET_ROOT: C:\Users\runneradmin\AppData\Local\Microsoft\dotnet

Package deployment using ZIP Deploy initiated.
Error: Failed to deploy web package to App Service.
Error: Deployment Failed with Error: Error: Failed to deploy web package to App Service.
Request-URI Too Long (CODE: 414)
App Service Application URL: http://web-app-test-eu-xxxxxxxxxxxxx-001.azurewebsites.net

I guess that it's due to the length of the app service name. Since the same workflow works well with other app services which have 1 character less in the name  For e.g below app services gets deployed successfully with same workflow.
http://web-app-dev-eu-xxxxxxxxxxxxx-001.azurewebsites.net
http://web-app-uat-eu-xxxxxxxxxxxxx-001.azurewebsites.net
http://web-app-prd-eu-xxxxxxxxxxxxx-001.azurewebsites.net

The error doesn't point the exact root cause so it's kind of very generic. Any pointer to fix ?

@venkatadorisala did you find a solution for this issue? I'm facing the same problem.

@JasonFreeberg - Please check this issue out, it is with Java on Windows.

@rabollin It doesn't seem like this is at all specific to Java. Their logs say that the request failed with Request-URI Too Long (CODE: 414). Looks like our frontends are rejecting the deployment request because the URL is too long?

Possibly related to #192 -- the action is sending the commit message in the deployment request. If the commit message is too long, then the deployment request fails.
PR from community: https://github.com/Azure/webapps-deploy/pull/226

Closing this issue, we have merged the fix of this issue and it will be released by 1st week of december.


Out of memory app crash when large number of UIImages loaded, despite limiting with .preload

If the user has a large number of photos in the selected folder (like >100, on my iPhone with 256GB memory), the app crashes without even calling didReceiveMemoryWarning(). My image source is the iOS Photos library, accessed via PHFetchResult.
Setting slideshow.preload = .fixed(offset: 5) makes no difference, although my understanding of the documentation and resolved Issue #42 is that this should fix the problem. I have confirmed that if I forcibly limit the number of UIImages loaded there is no crash.
Here is my relevant code:
`
func updateImageSourceArray(with _fetchResult: PHFetchResult?, size targetSize: CGSize) -> [ImageSource]? {
var _currentImageSourceArray: [ImageSource] = []
    if let assetCount = _fetchResult?.count {

            for index in 0..<assetCount {
            let asset = _fetchResult!.object(at: index)
            let options = PHImageRequestOptions()
            options.isSynchronous = true //wait until the full image is delivered
            imageManager.requestImage(for: asset, targetSize: targetSize, contentMode: .aspectFill, options: options, resultHandler: { image, info in
                guard image != nil else { return }
                _currentImageSourceArray.append(ImageSource(image: image!))
            })
        }
        return _currentImageSourceArray
    } else { return nil }
}

		// items needed for photo / image display
	fileprivate var currentImageSourceArray: [ImageSource]?
	fileprivate var preLoadOffset: Int = 5

    //set up the slideshowView
    slideshowView.preload = .fixed(offset: preLoadOffset)
    slideshowView.setImageInputs([]) //clear current set to force redraw, bug already reported as issue #338
    slideshowView.setImageInputs(currentImageSourceArray!)`

I am facing same issue, I have 42 images with large resolution. It takes around 1.5 GB of memory(RAM). Due to which app gets crash.

I had same issue. I found that, ImageSlideshow.presentFullScreenController method is reinitiating new ImageSlideshow object (under FullScreenImageSlideshowViewController) without properly initializing preload (default value = .all).
So even if we set that to .fixed(count: 5), it is not applied to FullScreenImageSlideshowViewController.slideshow.preload.
I was able to fix this by subclassing ImageSlideshow class, like this
class BulkImagSlideshow: ImageSlideshow {

@discardableResult
    open override func presentFullScreenController(from controller: UIViewController, completion: (() -> Void)? = nil) -> FullScreenSlideshowViewController {
        let fullscreen = FullScreenSlideshowViewController()
        fullscreen.pageSelected = {[weak self] (page: Int) in
            self?.setCurrentPage(page, animated: false)
        }

        /// This is added by hypermobileshop99
        fullscreen.slideshow.preload = preload
        fullscreen.initialPage = currentPage
        fullscreen.inputs = images
        ...
}


Cortona deprecation

It was recently announced that Cortona is being removed from WIn10/11 later this year (2023).  So it would make sense that at a minimum a note should be added here, and on similar pages, noting the deprecation of these APIs.  Also helpful might be information on whether inclusion of these APIs must be removed from code prior to the removal from the OS.

Document Details
⚠ Do not edit this section. It is required for learn.microsoft.com ➟ GitHub issue linking.

ID: bd86f224-3a44-d4ea-4267-b85b15ca122d
Version Independent ID: 08de37da-0403-7569-3c76-7f931f955a47
Content: Voice command (VCD) schemas - Windows UWP applications
Content Source: winrt-related-src/schemas/voicecommands/vcd-xml-schema-portal.md
Product: uwp
Technology: winrt-reference
GitHub Login: @Karl-Bridge-Microsoft
Microsoft Alias: kbridge

Thanks for calling this out. I consulted with the writer who worked on the Cortana content and his recommendatiion was to add this minimal boilerplate:
"The Cortana Skills Kit for consumer and enterprise, and the skills built on these platforms, have been deprecated."


Pin urllib3<2.0 to resolve requests lib issue for CI scripts [skip ci]

To fix latest py requests (minor version upgrade) but requires urllib3>2.0 issue
[2023-05-04T21:06:22.593Z]     raise ImportError(

[2023-05-04T21:06:22.593Z] ImportError: urllib3 v2.0 only supports OpenSSL 1.1.1+, currently the 'ssl' module is compiled with OpenSSL 1.0.2k-fips  26 Jan 2017. See: https://github.com/urllib3/urllib3/issues/2168

build

build


Firefox: disabling cookies breaks localforage

In Firefox, go to options -> privacy. Select "use custom settings for history". Untick "accept cookies from sites".
Now localStorage access or indexedDB.open throws SecurityError. This breaks anything using localforage, since it appears to call this on startup and throws an uncaught exception.
This is a known issue in Firefox (see: https://bugzilla.mozilla.org/show_bug.cgi?id=748620). However it has apparently been like this for a long time. I think localforage should work around it: ideally if indexedDB fails to open, it will fall back to an in-memory store that is simply thrown away when navigating away. This maximises the chance the page will continue to work as normal.

We could use the dummyStorageDriver that is used for the tests. We should probably refactor it to use the common utils that we now use.

We could possibly clone it and use a more "professional" looking name, like MemoryStorage or something.
@AshleyScirra have you tested the sessionStorageWrapper plugin in use case that you described?
I've been hearing that it works in some cases that localStorage fails.

Storing in memory has the advantage of avoiding any serializing to strings, and I can't remember but I thought Chrome Web Store apps dropped localStorage APIs in favour of IndexedDB/their own API. So I think a MemoryStorage driver would be useful to have as a workaround to Firefox's trigger-happy exceptions.

@AshleyScirra I totally agree with you, about in memory storage
BUT can you also answer to my previous question:

@AshleyScirra have you tested the sessionStorageWrapper plugin in the use case that you described?

No, it doesn't work: localforage throws an unhandled exception testing for IndexedDB support, which leaves localforage undefined, so you can't get as far as a defineDriver call.
The problem is Firefox throws SecurityError: The operation is insecure. on this line, which only tests for IndexedDB's presence:
var indexedDB = indexedDB || self.indexedDB || self.webkitIndexedDB || self.mozIndexedDB || self.OIndexedDB || self.msIndexedDB;

This is only referenced inside a try-catch statement further down, so it can be moved inside the try and it will detect it cannot use IndexedDB.
In this case all drivers fail to initialise and localforage is left with no driver. If I add the session storage driver, it incorrectly detects it can be used. Localforage will use it, but it too throws SecurityError upon any attempt to call sessionStorage.setItem/getItem!
So the only way to not have localforage completely broken in this case is to use an in-memory driver as a final fallback. Then despite Firefox's incredible awkwardness in this mode, data can be stored at least for the duration the page is open, maximising the chance the page can actually continue to function normally.

@AshleyScirra That's great!
Why don't you open a PR with this exact change?
PS: I can open a PRwith your suggested change tonight, if that's too much trouble for you.

I don't feel like I know this codebase well enough to implement a MemoryStorage driver, and moving the IndexedDB line only makes Firefox's failure case different, not better (loads with no driver rather than throwing exception). So could you make both changes?

Loading with no driver and rejecting the .ready() promise, is way better
than throwing an exception, since this allows developers to handle such
case and provide a custom driver.
I will open a PR for this tonight.
Regarding the in memory storage driver, I think we should track it in a
separate issue and get some feedback about it from @tofumatt.

--
Thodoris Greasidis
Computer, Networking &
Communications Engineer

Sounds good.
I’m okay with the in-memory driver as a general fallback for no cookies—it also might fix our private browsing problems, not sure though.


tag new release

The Cargo.lock on 1.0.0 is not locked correctly. Thus I can't package it for Nixpkgs.

Well, this might not be accurate. Not sure, I bumped to latest on main.

Hi @colemickens when something is wrong in some kind of way, I am happy to fix it, just drop a message (or issue) when required.

Thanks for such a nice message. So far, things are great. I have a generic OIDC client and it happily uses discovery, fetches keys, validates a token I got from fakeidp. Works great! And lovely to replace a ... similar, less functional nodejs tool.


[1.20] FTB Chunks Problem

Industrial Foregoing Version: 3.5.15
Titanium Version: 3.8.25
Wither Builder not working in FTB Chunks claim.

You need to activate "allow fake players" in your ftp rteams config


Allow setting socket parameters

At least timeout and keep_alive would be good to configure

At least as far as the socket timeout goes, this is controlled by the TCP client (impl TcpConnect) passed to the HttpClient. So, I cannot see a reason for this library to take a mutable ref of the client in order to offer this configuration.
In my case, setting a timeout looked like:
        // ...
        let mut tcp_client = embassy_net::tcp::client::TcpClient::new(stack, &client_state);
        tcp_client.set_timeout(Some(Duration::from_millis(3000)));

        let dns_client = embassy_net::dns::DnsSocket::new(stack);
        let tls_config = reqwless::client::TlsConfig::new(seed, &mut tls_read_buffer, &mut tls_write_buffer, TlsVerify::None);

        let mut http_client = reqwless::client::HttpClient::new_with_tls(&tcp_client, &dns_client, tls_config);


Too many uploads to this commit.

Hello,
It looks like recently you deployed some changes/rules about uploads.
Last successful nightly build on our project was on Friday 26th of February.  Since then the nightly and regular builds constantly fail on codecov reports upload.
Our CI runs many (hundreds) small jobs in parallel, and after each job we upload the reports to codecov.
I understand this might be an issue for you (thus the error Too many uploads to this commit), if so please guide us, document this new behaviour.
Thank you
Failure log
#!/bin/bash -eo pipefail
bash <(curl --retry 10 --retry-delay 5 --retry-max-time 60 -s https://codecov.io/bash) -Z

  _____          _
 / ____|        | |
| |     ___   __| | ___  ___ _____   __
| |    / _ \ / _` |/ _ \/ __/ _ \ \ / /
| |___| (_) | (_| |  __/ (_| (_) \ V /
 \_____\___/ \__,_|\___|\___\___/ \_/
                              Bash-20210226-7100762

==> git version <IP_ADDRESS> found
==> curl 7.29.0 (x86_64-redhat-linux-gnu) libcurl/7.29.0 NSS/3.44 zlib/1.2.7 libidn/1.28 libssh2/1.8.0
Protocols: dict file ftp ftps gopher http https imap imaps ldap ldaps pop3 pop3s rtsp scp sftp smtp smtps telnet tftp
Features: AsynchDNS GSS-Negotiate IDN IPv6 Largefile NTLM NTLM_WB SSL libz unix-sockets
==> Circle CI detected.
    project root: .
    Yaml not found, that's ok! Learn more at http://docs.codecov.io/docs/codecov-yaml
==> Running gcov in . (disable via -X gcov)
==> Python coveragepy not found
==> Searching for coverage reports in:
    + .
    -> Found 3 reports
==> Detecting git/mercurial file structure
==> Reading reports
    + ./lib/tasks/coverage.rake bytes=92
    + ./coverage/coverage.json bytes=13379
    + ./coverage/codecov-result.json bytes=283174
==> Appending adjustments
    https://docs.codecov.io/docs/fixing-reports
    + Found adjustments
==> Gzipping contents
        180K	/tmp/codecov.CECt51.gz
==> Uploading reports
    url: https://codecov.io
    query: branch=master&commit=06215fac9733963435a266c60ead0bde51b3dff8&build=202064&build_url=&name=&tag=&slug=3scale%2Fporta&service=circleci&flags=&pr=&job=0&cmd_args=Z
->  Pinging Codecov
https://codecov.io/upload/v4?package=bash-20210226-7100762&token=secret&branch=master&commit=06215fac9733963435a266c60ead0bde51b3dff8&build=202064&build_url=&name=&tag=&slug=3scale%2Fporta&service=circleci&flags=&pr=&job=0&cmd_args=Z
[ErrorDetail(string='Too many uploads to this commit.', code='invalid')]
400

Exited with code exit status 1

Does it fail on the first try already?
(I've thought it might be related to https://github.com/codecov/codecov-bash/issues/415 maybe - i.e. there are too many retries, although it worked (probably))

@blueyed No we do not loop, we just try once per job (but implying multiple times per commit)
Do you know if there are hard limits of how many uploads per commit? I suppose it is this problem. Did you encounter that too or just because you receive a false negative bash exit code?
@drazisil-codecov or @dorianamouroux (sorry to ping you but it seems you could help us or orient us where to report issues, again my apologies if it is not appropriate)
How could we work around that knowing we have around 100 jobs to process and each of the job reports to codecov?


record channel?

A simple question: but can we record the show?
either simply live or based on EPG? The recording could be either locally on the flash card or on the local network
thanks

Hello Florent,
I know about this project called IPTV Recorder, but it's not up to date. Never tried it on Matrix though. If you hear about a solution, more linked to IPTV Simple globally than this Orange addon, I would love it!

thanks